[PATCH] D130919: [MRI] isConstantPhysReg should also check if the register is clobbered by a RegMask

Tue Nov 1 19:06:11 PDT 2022

Carrot added a comment.

@foad, thanks a lot for reviewing AMDGPU changes!

================
Comment at: llvm/lib/CodeGen/MachineCSE.cpp:280
   const MachineRegisterInfo &MRI = MF.getRegInfo();
-  return TRI.isCallerPreservedPhysReg(Reg, MF) ||
+  return TRI.isCallerPreservedPhysReg(Reg, MF) || TII.isIgnorableUse(MO) ||
          (MRI.reservedRegsFrozen() && MRI.isConstantPhysReg(Reg));
----------------
foad wrote:
> Can you please split this part out into a separate patch for review? It causes significant codegen changes for AMDGPU and we need to be sure that the changes are OK.
It is split to D137222. I also did an analysis for it.

================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/udiv.i64.ll:1
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -global-isel -amdgpu-codegenprepare-disable-idiv-expansion=1 -mtriple=amdgcn-amd-amdpal -denormal-fp-math-f32=preserve-sign -mattr=+mad-mac-f32-insts < %s | FileCheck -check-prefixes=CHECK,GISEL %s
----------------
foad wrote:
> I don't understant why these diffs are included in the patch. Can you double check? I don't think your patch affects codegen for any of:
> ```
>   LLVM :: CodeGen/AMDGPU/GlobalISel/udiv.i64.ll
>   LLVM :: CodeGen/AMDGPU/GlobalISel/urem.i64.ll
>   LLVM :: CodeGen/AMDGPU/sgpr-control-flow.ll
> ```
Will investigate it.

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

https://reviews.llvm.org/D130919