[PATCH] D84100: [ARM] Optimize immediate selection

Ben Shi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 11 03:59:10 PDT 2020


benshi001 added a comment.

In D84100#2209387 <https://reviews.llvm.org/D84100#2209387>, @samparker wrote:

> If the logic isn't needed, then yes, I'd recommend removing. Currently I'm seeing logic added for Thumb2 opcodes so I would expect some changes and testing and if that isn't case, then I guess the changes aren't needed. Looks like the ORR and EOR paths also need to be tested?

Sorry, I made a mistake, the changes to thumb are necessary, since

  BuildMI(*UseMI.getParent(), UseMI, UseMI.getDebugLoc(), get(NewUseOpc),
           NewReg)
       .addReg(Reg1, getKillRegState(isKill))
       .addImm(SOImmValV1)
       .add(predOps(ARMCC::AL))
       .add(condCodeOp());
  UseMI.setDesc(get(NewUseOpc));

is changed to

  BuildMI(*UseMI.getParent(), UseMI, UseMI.getDebugLoc(), get(NewUseOpc1),
         NewReg)
     .addReg(Reg1, getKillRegState(isKill))
     .addImm(SOImmValV1)
     .add(predOps(ARMCC::AL))
     .add(condCodeOp());
  UseMI.setDesc(get(NewUseOpc2));

This piece of code is in the critical path, all cases will walk though it.

It is necessary to substitute NewUseOpc with NewUseOpc1 & NewUseOpc2.

I will upload a new patch later, with tests of orr and eor.


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

https://reviews.llvm.org/D84100



More information about the llvm-commits mailing list