Ping?

The new tests are failing in my Release build:

Failing Tests (3):
    LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-and.mir
    LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-or.mir
    LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-xor.mir

The failures all look like:

/home/jayfoad2/git/llvm-project/llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-and.mir:120:12: error: WAVE64: expected string not found in input
 ; WAVE64: [[COPY:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
           ^
<stdin>:375:2: note: scanning from here
 %0:sreg_32_xm0 = COPY $sgpr0
 ^

Rewrite to remove duplicated code.

Add a negative test for fmul without fast math flags.

In D64713#1585110, @xbolva00 wrote:

b) yes, one more test never hurts

In D64713#1585078, @xbolva00 wrote:

I think we need a few more tests here

a) vector tests
b) 'extra use' tests
c) negative tests

Add a vector test.

In D64713#1585080, @xbolva00 wrote:

Not sure how other reviewers, but I consider the use of 'C' as confusing here, since 'C' is usually a constant. I would recommend to avoid 'C', if it is not a constant.

Edit: As I see, original code also used 'C', so feel free to ignore me :)

Count instructions instead of uses.

Remove unnecessary check for debug operands.

In D64497#1581712, @hakzsam wrote:

This change introduced CTS regressions with RADV like dEQP-VK.spirv_assembly.instruction.graphics.float16.arithmetic_3.atan2_frag and friends.
Can you investigate ?

In D64497#1581712, @hakzsam wrote:

This change introduced CTS regressions with RADV like dEQP-VK.spirv_assembly.instruction.graphics.float16.arithmetic_3.atan2_frag and friends.
Can you investigate ?

Add a DPP combiner test that would have caught the problem.

Count non-debug use operands of non-debug instructions instead of using
a SmallPtrSet.

I'll add tests tomorrow.

Address some review comments.

In D64497#1578444, @arsenm wrote:

We still need more work to handle the integer cases in the complex pattern.

In D64393#1578090, @vpykhtin wrote:

Sorry, there is a code for returnining unique instr, so we can use the count:

defusechain_instr_iterator &operator++() {          // Preincrement
      assert(Op && "Cannot increment end iterator!");
      if (ByOperand)
        advance();
      else if (ByInstr) {
        MachineInstr *P = Op->getParent();
        do {
          advance();
        } while (Op && Op->getParent() == P); // <- removes duplicates

In D64393#1578035, @vpykhtin wrote:

Jay, I'm ok with your patch, just a note that the prt set can be avoided if we count the number of use instructions and use the count to limit the scan.

Try a different approach.

New helper function getIdentityValueForAtomicOp.

Fix identity for signed min.

See also comments here about whether to include the shift by 3: https://reviews.llvm.org/D57737#1392824

In rL363675#668425, @arsenm wrote:

I rather strongly dislike this "check all uses" API. I would expect this fold to only be done for hasOneNonDBGUse anyway, but at the time didn't want to spend time fixing the DPP pas

I guess the point of "using a set to track seen uses" was for exactly this case: when no use is specified, but all uses are in the same BB, we want to scan as far as the last use, but no further. But perhaps this could be implemented with a count of uses instead of a set.

Add i64 tests.

In rL363675#668365, @arsenm wrote:

We don't want to rely on kill flags

This commit stopped the DPP combiner from working on code sequences generated by the atomic optimizer. For example:

declare i32 @llvm.amdgcn.workitem.id.x()
@local_var32 = addrspace(3) global i32 undef, align 4
define amdgpu_kernel void @add_i32_varying() {
entry:
  %lane = call i32 @llvm.amdgcn.workitem.id.x()
  %old = atomicrmw add i32 addrspace(3)* @local_var32, i32 %lane monotonic
  ret void
}

With llc -print-before-all -debug-only=gcn-dpp-combine -march=amdgcn -mcpu=gfx900 -amdgpu-atomic-optimizations < test.ll I get:

Add a test case, and change the tests to run on gfx9 because it needs
add-no-carry instructions.

In D64207#1570526, @vpykhtin wrote:

Right, this is hard to follow even for me :). 3rd operand is src1_modifiers, you can use a junk value for this to check whether the DPP combiner don't crash and don't combine it.

In D64207#1570456, @vpykhtin wrote:

I think modifiers are checked correctly by the existing code, but can you add a test for e64 encodings into dpp_combine.mir similar to what is under "check for floating point modifiers" comment?

Typo fixes have been committed separately.

Add fcmp and intrinsic tests.

Add fneg test.

Hoping for review from someone outside AMD.

In D63375#1544851, @hfinkel wrote:

The test case does an (out of bounds) load from a global constant with type <3 x float>.

Do you need an out-of-bounds access to trigger the problem?

Jay, could you please check it on your PPC box?