[PATCH] D150347: [AMDGPU] Improve abs modifier usage

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp:223-225
+  // Early opt-out as we don't want to operate on undef or poison values.
+  if (Intrinsic && hasUndefOrPoisonOperand(*Intrinsic))
+    return false;
----------------
foad wrote:
> Why do you need this special case at all? And why only for intrinsics?
We don't really need to handle it, but it is useless to clone (for instance) an fabs call with an undef operand.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp:232-234
+    Instruction *UI = cast<Instruction>(U);
+    if (!UI)
+      continue;
----------------
foad wrote:
> All users of Instructions are Instructions, so this cast will always succeed. (And anyway if you want a cast that can fail, you need to use `dyn_cast`.)
Thanks. I have overlooked that one.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULateCodeGenPrepare.cpp:253
+    if (Intrinsic) {
+      // Don't generate fabs(fabs(x)) calls.
+      if (auto *UII = dyn_cast<IntrinsicInst>(UI);
----------------
foad wrote:
> I don't think you need a special case for this. Any fabs(fabs(x)) - even in different basic blocks - should already have been simplified by generic IR optimizations.
Makes sense. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150347