[PATCH] D142084: [RFC][X86][MemFold] Upgrade the mechanism of auto-generated Memory Folding Table

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/utils/TableGen/X86FoldTablesEmitter.cpp:442
+    Result.CannotUnfold = BaseRegInst->isMoveReg ? true : Result.CannotUnfold;
+  } else if (RegInstr->isMoveReg && Result.IsStore)
     Result.CannotUnfold = true;
----------------
yubing wrote:
> yubing wrote:
> > skan wrote:
> > > Why do we need `Result.IsStore` here?
> > load such as VMOVAPDZ128rm will never be unfolded.
> > if VMOVAPDZ128rm 's memoperand is invariant, then VMOVAPDZ128rm  will be hoisted directly and won't be unfolded into rm+rr
> > if VMOVAPDZ128rm 's memoperand is not invariant, we stop doing unfolding since compiler find it is a simple load.
> > ```
> >   // First check whether we should hoist this instruction.
> >   if (!IsLoopInvariantInst(*MI) || !IsProfitableToHoist(*MI)) {
> >     // If not, try unfolding a hoistable load.
> >     MI = ExtractHoistableLoad(MI);
> >     if (!MI) return false;
> >   }
> > ```
> > ```
> > MachineInstr *MachineLICMBase::ExtractHoistableLoad(MachineInstr *MI) {
> >   // Don't unfold simple loads.
> >   if (MI->canFoldAsLoad())
> >     return nullptr;
> > 
> > ```
> > @craig.topper , does line429~439 make sense to you as well?
> @craig.topper i think my analysis of VMOVAPDZ128rm is only for LICM. but for other passes in codegen, is it possible for VMOVAPDZ128rm to be unfolded?
The other 2 places I know that unfold are TwoAddressInstructionPass and SelectionDAG->MachineIR

TwoAddressInstuctionPass should only happen for instructions with tied source and dest. That doesn't apply to moveReg.

I think SelectionDAG->MachineIR case happens if we need to duplicate an instruction that has an EFLAGS def. If the EFLAGS are used by two instructions and EFLAGS is clobbered in between them. We need to duplicate the flag producing instruction to satisfy the second user. But we can't duplicate the folded load so we have to unfold. That doesn't apply to moveReg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142084