[PATCH] D140952: Teach the AArch64 backend to materialize immediates using a pair of ORR-immediateinstructions.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello. Feel free to add reviewers to a patch, otherwise people might not see it or think it is still WIP. The patch looks like a nice addition.

Can you clang-format the patch? I don't believe that ifs and returns should be on the same line.

There is another use of expandMOVImm in the MachineCombiner. Is that OK because it only generated single instructions? It should possibly just be generating a MOVi32imm/MOVi64imm instead.



================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandImm.cpp:308
+
+  // Remove all bits that can are set by this mask.
+  uint64_t RemainingBits = RotatedBits & ~MaximalImm1;
----------------
Remove can


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandImm.cpp:345
+
+// Attempt to expand an immediate as a pair of ANDs of logical immediates.
+static bool tryAndOfLogicalImmediates(uint64_t UImm,
----------------
"as an AND of an OR"? or a maybe explain the inverse of the immediate thing. I can see why this would work, but find it difficult to follow - it would be good to add a longer comment explaining how it works.


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:169
+    case AArch64::ANDXri:
+      if (I->Op1 == 0) {
+        MIBS.push_back(BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(I->Opcode))
----------------
Can this happen?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140952