[PATCH] D116053: [MachineSink] Allow sinking of constant or ignorable physreg uses

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D116053#3255695 <https://reviews.llvm.org/D116053#3255695>, @sebastian-ne wrote:

> This change seems to sink v_cmp instructions, which creates different results if the exec mask changed and that makes several Vulkan tests fail.
> I put a reproducer here: https://gist.github.com/Flakebi/fd1d91a806b60ec330e9f61e19fe62ac
> Compile with `llc -mtriple=amdgcn--amdpal -mcpu=gfx1010 -verify-machineinstrs -start-before=machine-sink -stop-after=machine-sink PipelineVsFs_0xDD57C231E25DA514.mir -o PipelineVsFs_0xDD57C231E25DA514-after.mir`
> and the `%104:sreg_64 = V_CMP_NE_U32_e64 %89, %101, implicit $exec` instruction will be sunk from bb.5 into bb.6. For reference, the pipeline is from the dEQP-VK.subgroups.arithmetic.framebuffer.subgroupexclusiveadd_float_vertex CTS test.

Does this help?

  diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  index 88996f455227..0678ceeeea21 100644
  --- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  +++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  @@ -130,10 +130,29 @@ bool SIInstrInfo::isReallyTriviallyReMaterializable(const MachineInstr &MI,
     return false;
   }
  
  +static bool readsExecAsData(const MachineInstr &MI) {
  +  if (MI.isCompare())
  +    return true;
  +
  +  switch (MI.getOpcode()) {
  +  default:
  +    break;
  +  case AMDGPU::V_READFIRSTLANE_B32:
  +  case AMDGPU::V_CNDMASK_B64_PSEUDO:
  +  case AMDGPU::V_CNDMASK_B32_dpp:
  +  case AMDGPU::V_CNDMASK_B32_e32:
  +  case AMDGPU::V_CNDMASK_B32_e64:
  +  case AMDGPU::V_CNDMASK_B32_sdwa:
  +    return true;
  +  }
  +
  +  return false;
  +}
  +
   bool SIInstrInfo::isIgnorableUse(const MachineOperand &MO) const {
     // Any implicit use of exec by VALU is not a real register read.
     return MO.getReg() == AMDGPU::EXEC && MO.isImplicit() &&
  -         isVALU(*MO.getParent());
  +         isVALU(*MO.getParent()) && !readsExecAsData(*MO.getParent());
   }
  
   bool SIInstrInfo::areLoadsFromSameBasePtr(SDNode *Load0, SDNode *Load1,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116053