[PATCH] D102096: [DAGCombiner] Fix DAG combine store elimination, different address space.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D102096#2749834 <https://reviews.llvm.org/D102096#2749834>, @RKSimon wrote:

> In D102096#2749297 <https://reviews.llvm.org/D102096#2749297>, @hgreving wrote:
>
>> In D102096#2748611 <https://reviews.llvm.org/D102096#2748611>, @spatel wrote:
>>
>>> Code changes LG. 
>>> I'd still prefer for the tests to go in first, so we have a regression test record of the miscompiles. Let me know if the suggestion was missed or isn't clear.
>>
>> I did miss this comment. Could you clarify, do you want me to commit the failing test, or a passing version with a FIXME?
>
> @spatel meant the passing version (current trunk codegen) with a FIXME - you then rebase so this patch shows the codegen diffs - its easier to handle if you auto generate the checks with the update scripts.

Right. It would be nice to commit the baseline (current) output - it will look something like this:

  define i32 @copy_fs_same() {
  ; CHECK-LABEL: copy_fs_same:
  ; CHECK:       # %bb.0:
  ; CHECK-NEXT:    movl 1, %eax
  ; CHECK-NEXT:    retl
    %t0 = load i32, i32* inttoptr (i64 1 to i32*), align 4
    store i32 %t0, i32 addrspace(257)* inttoptr (i64 1 to i32 addrspace(257)*), align 4
    ret i32 %t0
  }

One more test question - is it possible to add a test that exercises the indexed address code path?
Something like this might do it?

  define void @output_fs_same_indexed(i32 %v, i32* %b) {
  ; CHECK-LABEL: output_fs_same:
  ; CHECK:       # %bb.0:
  ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %eax
  ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %ecx
  ; CHECK-NEXT:    movl %eax, 168(%ecx)
  ; CHECK-NEXT:    retl
    %p = getelementptr i32, i32* %b, i64 42
    %pa = addrspacecast i32* %p to i32 addrspace(257)*
    store i32 %v, i32* %p, align 4
    store i32 %v, i32 addrspace(257)* %pa, align 4
    ret void
  }


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

https://reviews.llvm.org/D102096