freaknbigpanda wrote: > I'm not sure about forking the algorithm into two separate implementations... > why did you decide to go this route? I can see some reasons why you might, > but I'd like to start from your explanation. > > How hard would it be to modify the 24 algorithm to add narrow checks to > detect cases that would be handled differently by the 23 algorithm?
I thought it there were too many differences in behaviour and thus trying to merge the two classification algorithms was too risky and would result in buggy, hard to maintain, hard to read code. I was worried that if we tried to do that we would inadvertently create ABI breaks in the legacy classifier which would break platforms that require a stable ABI (like PlayStation). I wanted to create a new cleaner classification algorithm that is easier to reason about and maintain, and one that faithfully implements the psABI classification algorithm. All that being said I took another look at this and there was some stuff that was common to both classifiers that could be factored out, reducing duplicated code. So I went ahead and did this, I have pushed a new diff. Now the classifiers use the same factored out functions to classify the following types: - BuiltinType via getBuiltinTypeClassification - ComplexType via getComplexTypeClassification - BitIntType via getBitIntTypeClassification - Member-pointer span classification through getClassPairForSpan - Some common pre-checking code for Vectors and Arrays I think this makes the implementations easier to follow. Most remaining separate code either handles types that had no explicit classification in the legacy algorithm, such as AtomicType, OverflowBehaviorType, PipeType, and MatrixType, or implements the different vector and aggregate classification used for RecordType, VectorType, and ConstantArrayType. In particular, faithfully implementing the psABI for values larger than 128 bits requires tracking and merging every eightbyte, which the legacy algorithm cannot do. https://github.com/llvm/llvm-project/pull/221746 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
