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

[Hendrik Greving via Phabricator via llvm-commits]

hgreving added a comment.

In D102096#2750327 <https://reviews.llvm.org/D102096#2750327>, @spatel wrote:

> 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
>   }

WB https://reviews.llvm.org/D102243 ?
Let me add the suggested test changes.


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

https://reviews.llvm.org/D102096