[PATCH] D93370: [PowerPC] Add new infrastructure to select load/store instructions, update P8/P9 load/store patterns.

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

> The primary motivation is that the current implementation of selecting load/stores is dependent on the ordering of patterns in TableGen.

I think this also can be resolved by `AddedComplexity` in td files? Is it convenient to solve the issue in D91279 <https://reviews.llvm.org/D91279> when the worst case happens in instruction selection for load/store. With legacy infra, we have to select a xform with one zero register operand for worst case. But seems on P10 <https://reviews.llvm.org/P10>, we prefer a dform with 0 offset, it is good for some linker opt. That issue does not have a simple fix with legacy infra because the worst case is handled in select xform only function, we must select a xform, so we must use zero register. Could it be solved easier under the new infra? Thanks.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:698
+
+      // Subtarget features.
+      MOF_SubtargetNoP9 = 1 << 26,
----------------
amyk wrote:
> shchenz wrote:
> > The following features should not be associated to one specific memory operation? Should we add them into the flag set of each memory operation in `computeMOFlags`? Is it possible to get the sub-target info when we do am selection after we get all flags?
> I apologize, I think I do not quite follow. If you could clarify, that would be great.
> 
> Are you suggesting that these should not be flags, but more of Subtarget checks after the flags are computed?
> 
> I think it can be useful to store the subtarget information in the set of flags is that we can easily know which instructions we can produce on P9 and P10. 
> 
> I was thinking for instance, if we have flags set for `SubtargetP9`, `ScalFlt` and `RPlusSImm16Mult4` (register + signed 16 bit immediate, multiple of 4), then we know we can generate the DS-Form (corresponding to `DFLOAD` pseudoinstruction. Or, when we have `PPC::MOF_SubtargetP10` and `PPC::MOF_RPlusSImm34` (signed 34-bit immediate), we can know we can generate prefixed load/stores.
> 
> 
> Are you suggesting that these should not be flags, but more of Subtarget checks after the flags are computed?
Yes, that was my first thought. For normal load/store instructions before ISEL, we can get type, address info(zext/sext/imm) from the instruction itself. But sub-target is not loads/stores characteristic. we can check the sub-target info when we do the address mode selection when we have other flags. But I am ok if making the sub-target as a flag for load/store for implementation convenience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93370