[PATCH] D141258: [AArch64] Fix crash for expandMOVImm

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandImm.cpp:343
 
-  assert(BitSize == 64 && "All 32-bit immediates can be expanded with a"
+  assert(BitSize > 32 && "All 32-bit immediates can be expanded with a"
                           "MOVZ/MOVK pair");
----------------
Allen wrote:
> Allen wrote:
> > efriedma wrote:
> > > 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.
> > Oh, Thanks for your confirm
> 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.
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.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141258/new/

https://reviews.llvm.org/D141258