[PATCH] D86949: [ARM] Fix so immediates and pc relative checks

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 1 12:52:40 PDT 2020


efriedma added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:786
             IsSoImm = true;
+            {
+              unsigned CPI = I.getOperand(op).getIndex();
----------------
Standard code style in LLVM is something like `case ARM::LEApcrelJT: {`


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:788
+              unsigned CPI = I.getOperand(op).getIndex();
+              MachineInstr *CPEMI = CPEMIs[CPI];
+              const Align CPEAlign = getCPEAlign(CPEMI);
----------------
Does this do something sane for ARM::LEApcrelJT?  I don't think the operand for that is a CPI.


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:796
+                // which is safe for 2-byte constants with 2-byte alignment.
+                Scale = 1;
+            }
----------------
Instead of checking "if (LogCPEAlign >= 2)", could we just set "Scale = CPEAlign;"?  I don't care that much about the quality of lowering for 16-bit constant pool entries, but the intent would be more clear.


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

https://reviews.llvm.org/D86949



More information about the llvm-commits mailing list