[PATCH] D96605: AMDGPU/GlobalISel: Remove redundant G_FCANONICALIZE

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9649
 
+bool SITargetLowering::isCanonicalized(Register Reg, MachineFunction &MF,
+                                       unsigned MaxDepth) const {
----------------
I don't think this really belongs in SITargetLowering, but I don't have a better suggestion for now


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9655-9658
+  if (Opcode == AMDGPU::G_FCANONICALIZE)
+    return true;
+
+  if (Opcode == AMDGPU::G_FCANONICALIZE) {
----------------
Duplicated / dead path for fcanonicalize. This meant G_FCONSTANT


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:9671
+  case AMDGPU::G_FMAXNUM_IEEE: {
+    if (Subtarget->supportsMinMaxDenormModes() ||
+        denormalsEnabledForType(MRI.getType(Reg), MF))
----------------
We shouldn't actually treat these generic instructions differently based on the subtarget, but I guess that's an existing problem


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fcanonicalize.mir:169
+    $vgpr0 = COPY %3(s32)
+...
----------------
Needs tests for the G_FCONSTANT cases


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

https://reviews.llvm.org/D96605