[PATCH] D30786: [GlobalISel] LegalizerHelper: Lower (G_FSUB X, Y) to (G_FADD X, (G_FNEG Y))

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:581
+    MIRBuilder.buildInstr(TargetOpcode::G_FNEG).addDef(Neg).addUse(RHS);
+    MIRBuilder.buildInstr(TargetOpcode::G_FADD)
+        .addDef(Res)
----------------
qcolombet wrote:
> FNEG could be illegal and thus it may be legalized into FSUB.
> With that in mind, that means we could theoretically end up with an endless loop.
> How do you think we should address that?
> Note: I don't expect we solve the problem as part of this commit, but we would need a solution at some point. 
My feeling is that it's probably better not to try to define generically how G_FSUB should be lowered if the target doesn't support it.
This patch in the generic code indeed assumes that G_FADD and G_FNEG will be legal when G_FSUB isn't.
I just think it's better not to make these assumptions in the generic code and just move this to target specific code.


https://reviews.llvm.org/D30786