[PATCH] D109963: [AArch64] Split bitmask immediate of bitwise AND operation
JinGu Kang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 23 04:41:41 PDT 2021
jaykang10 added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:52
+ void getAnalysisUsage(AnalysisUsage &AU) const override {
+ AU.addRequired<MachineLoopInfo>();
+ MachineFunctionPass::getAnalysisUsage(AU);
----------------
dmgreen wrote:
> Can this preserve CFG, which should help it preserve some analysis passes
Yep, I think so. Let me add `AU.setPreservesCFG`.
================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:66
+bool AArch64MIPeepholeOpt::visitAND(
+ MachineInstr &MI, unsigned regSize,
+ SmallVectorImpl<MachineInstr *> &ToBeRemoved) {
----------------
dmgreen wrote:
> Can this use regsize == sizeof(T), to simplify it a little
Yep, let me update it.
================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:81
+ MachineLoop *L = MLI->getLoopFor(MBB);
+ if (L && !L->isLoopInvariant(MI))
+ return false;
----------------
dmgreen wrote:
> Does it matter if MI is loop invariant? As opposed to the MOV.
Let's say `MOV` is in preheader and `AND` is in the loop as below.
```
preheader:
%1:gpr32 = MOVi32imm 2098176
%2:gpr32 =
...
loop:
...
%3:gpr32 = ANDWrr killed %2:gpr32, %1:gpr32
```
In above example, AND is loop invariant and we can split the constant of MOV.
```
preheader:
%1:gpr32 = MOVi32imm 2098176
...
loop:
...
%2:gpr32 = PHI
%3:gpr32 = ANDWrr killed %2:gpr32, %1:gpr32
```
In above example, the AND is not loop invariant and we can not split the constant of MOV because the first AND needs %2 and the AND is outside loop.
================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:129
+ // If mov instruction has one use, delete it.
+ if (MRI->hasOneUse(DefMI->getOperand(0).getReg()))
+ ToBeRemoved.push_back(DefMI);
----------------
dmgreen wrote:
> Should this transform be done with multiple uses on the mov?
I thought there could be other opportunities with two bitmask immediate and other bitwise operations like `OR`. If possible, I would like to keep the MOV for the potential opportunities.
================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h:342
+template <typename T>
+static inline bool isValidAndSplitBitmaskImm(T Imm, unsigned regSize) {
+ assert((regSize == 32 || regSize == 64) &&
----------------
dmgreen wrote:
> This can move into the peephole optimization pass now, which might allow it to be simplified somewhat.
This function could be used on other places. If possible, I would like to keep this check here conservatively.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109963/new/
https://reviews.llvm.org/D109963
More information about the llvm-commits
mailing list