[PATCH] D134336: [AArch64] Lower multiplication by a constant int to madd

[Allen zhong via Phabricator via llvm-commits]

Allen marked 2 inline comments as done.
Allen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5922
+        BuildMI(MF, Root.getDebugLoc(), TII->get(MovI->Opcode), NewVR)
+            .addImm(MovI->Op1)
+            .addImm(MovI->Op2);
----------------
efriedma wrote:
> Allen wrote:
> > efriedma wrote:
> > > Sorry, missed an issue here the first time I looked at this...
> > > 
> > > The current complete set of opcodes that immediate expansion can produce is listed in AArch64ExpandPseudo::expandMOVImm .  If the opcode is movz/movn, there are two immediate operands.  But the opcode can also be ORRWri/ORRXri; in that case, there a register operand (wzr/xzr), and an immediate operand.
> > > 
> > > I think this comes up for something like `int f(int a) { return a * 44 + 16773120; }`.
> > Despite  the fact that movz/movn have two immediate operands, but it will generate a mov as alias, which only has one  immediate operand.
> > see the following code frag in AArch64_IMM::expandMOVImm.
> > ```
> >   // Prefer MOVZ/MOVN over ORR because of the rules for the "mov" alias.
> >   if ((BitSize / 16) - OneChunks <= 1 || (BitSize / 16) - ZeroChunks <= 1) {
> >     expandMOVImmSimple(Imm, BitSize, OneChunks, ZeroChunks, Insn);
> >     return;
> >   }
> > ```
> The first instruction returned by expandMOVImm is always "mov", yes.  "mov" is an alias for one of three instructions: movz, movn, and orr.  (See section C3.3.4 in the ARM architecture reference.)
> 
> As far as I can tell, the operands you're adding are correct for movz and movn, but not orr.
Done, revert to use orr if valid,thanks


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

https://reviews.llvm.org/D134336