llvm dev - Mar 2020 - [InstCombine] Addrspacecast and GEP assumed commutative

It appears that this behaviour of InstCombine is at least somewhat intended, as
there are several tests that fail after the change mentioned before: cast.ll,
gep-addrspace.ll and getelementptr.ll in test/Transforms/InstCombine.
I feel a little uncomfortable applying the patch after knowing this, and
removing those tests doesn't seem like a great solution.
What are your thoughts?

For now, I think I can work around my original issue since the CUDA driver seems
to optimise away the additional ADD in most cases.

________________________________
Van: Matt Arsenault <whatmannerofburgeristhis at gmail.com> namens Matt
Arsenault <arsenm2 at gmail.com>
Verzonden: maandag 23 maart 2020 19:05
Aan: Thomas Faingnaert <thomas.faingnaert at hotmail.com>
CC: Johannes Doerfert <johannesdoerfert at gmail.com>; llvm-dev at
lists.llvm.org <llvm-dev at lists.llvm.org>
Onderwerp: Re: [llvm-dev] [InstCombine] Addrspacecast and GEP assumed
commutative



On Mar 23, 2020, at 10:52, Thomas Faingnaert via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

Would replacing this by stripPointerCastsSameRepresentation be an acceptable
solution, or do other parts of LLVM rely on this reordering?
If not, I can send a patch in Phabricator to address this issue.

This is probably the right solution. Nothing should be relying on this anyway

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200324/3b0c08ed/attachment.html>

> On Mar 24, 2020, at 08:50, Thomas Faingnaert <thomas.faingnaert at
hotmail.com> wrote:
> 
> It appears that this behaviour of InstCombine is at least somewhat
intended, as there are several tests that fail after the change mentioned
before: cast.ll,gep-addrspace.ll and getelementptr.ll in
test/Transforms/InstCombine.
> I feel a little uncomfortable applying the patch after knowing this, and
removing those tests doesn't seem like a great solution.
> What are your thoughts?
These are checking for the current behavior, it’s only natural tests would need
updating for this change. You should just update them to show the new result

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200324/16bf6f64/attachment.html>

Hi Thomas,

We hit on this issue internally with GEP and addrspacecast being reordered.
Curious if you have a patch out for review?

Thanks,
Anna
On Mar 24, 2020, at 10:23 AM, Matt Arsenault via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:



On Mar 24, 2020, at 08:50, Thomas Faingnaert <thomas.faingnaert at
hotmail.com<mailto:thomas.faingnaert at hotmail.com>> wrote:

It appears that this behaviour of InstCombine is at least somewhat intended, as
there are several tests that fail after the change mentioned before:
cast.ll,gep-addrspace.ll and getelementptr.ll in test/Transforms/InstCombine.
I feel a little uncomfortable applying the patch after knowing this, and
removing those tests doesn't seem like a great solution.
What are your thoughts?

These are checking for the current behavior, it’s only natural tests would need
updating for this change. You should just update them to show the new result

-Matt
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200331/f9528b4f/attachment-0001.html>