Details
- Reviewers
zatrazz efriedma - Commits
- rG90b60f9cd6ea: [AArch64] Fix crash for expandMOVImm
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/AArch64/AArch64ExpandImm.cpp | ||
---|---|---|
343 ↗ | (On Diff #487301) | This algorithm doesn't work correctly if BitSize isn't 32 or 64. Probably the code that's creating such an immediate should be fixed. |
llvm/lib/Target/AArch64/AArch64ExpandImm.cpp | ||
---|---|---|
343 ↗ | (On Diff #487301) | Oh, Thanks for your confirm |
llvm/lib/Target/AArch64/AArch64ExpandImm.cpp | ||
---|---|---|
343 ↗ | (On Diff #487301) | hi @efriedma For all immediate numbers whose bit size is smaller than or equal to 64, at least four registers can be used for concatenation, ie, the processing of AArch64_IMM::expandMOVImm. Therefore, the current i53 immediate number can be also processed here (may not be the most efficient). Would you please show more detail about where is the algorithm error? thanks. |
llvm/lib/Target/AArch64/AArch64ExpandImm.cpp | ||
---|---|---|
343 ↗ | (On Diff #487301) | processLogicalImmediate assumes BitSize is a power of two. Loops involving BitSize assume they iterate either twice or four times. Probably other things. (You could theoretically stick something like "BitSize = BitSize <= 32 ? 32 : 64;" at the beginning of the function, and that would sort-of work, but calling the function with a BitSize that doesn't correspond to the width of a register doesn't make sense.) |
I'd rather stick the bitsize check in the caller, AArch64TargetLowering::isMulAddWithConstProfitable; without that context, it isn't clear why the bitwidth should be modified this way.
Stick the bitsize check in the caller according comment, AArch64TargetLowering::isMulAddWithConstProfitable