[PATCH] D133340: [PowerPC][GISel]select floating point constant from TOC

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCInstructionSelector.cpp:292-296
+  auto LoadInst = BuildMI(*I.getParent(), I, I.getDebugLoc(),
+                          TII.get(TargetOpcode::G_LOAD), DstReg)
+                      .addReg(AddrReg)
+                      .addMemOperand(MMO);
+
----------------
shchenz wrote:
> arsenm wrote:
> > This should have been done in the legalizer, not the selector
> I am not quite sure I understand your suggestion here. Do you mean we set action for G_FCONSTANT as custom for S32 and S64 in PPCLegalizerInfo? For my understanding, legalizer should do legalization for the types that are illegal for some opcodes on a certain target, right? float and double are both valid types on PPC. So not sure why we need to legalize them or do I misunderstand your comment? Thank you!
Essentially yes, but not quite like that. I think the default lower action for G_CONSTANT/G_FCONSTANT should look like what the DAG does, which produces a constant pool load. You will need to introduce a new generic G_CONSTANT_POOL instruction.

So you would set your action to lower and implement this in the generic LegalizerHelper. Even if float and double are "legal types", the fact that you are replacing these with load indicates constants are not legal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133340