[llvm] r347502 - [DAG] consolidate shift simplifications

[Sanjay Patel via llvm-commits]

Ah, so some transform replaces a shift operand which produces a bogus
shift, but that shift isn't visited before the xor from:
https://reviews.llvm.org/rL347478
...then we still have crash potential. So it's like the relationship
between instsimplify and instcombine. Unless we guarantee that we've run
simplifications before the more general combines, we have to be aware of
bogus instructions. And since we don't run DAGCombiner to fixpoint, there's
actually optimization potential from lifting all 'DAG simplifies' to some
common place and running that before putting nodes on the worklist.

As a first experiment, I put a 'simplifyShift' call into
DAGCombiner::AddUsersToWorklist(), and I get 4 codegen test failures:
Failing Tests (4):
    LLVM :: CodeGen/AMDGPU/load-constant-i16.ll
    LLVM :: CodeGen/AMDGPU/load-global-i16.ll
    LLVM :: CodeGen/X86/pr32588.ll
    LLVM :: CodeGen/X86/pr34855.ll

I didn't look at the AMDGPU diffs. For x86, we have what appears to be 1
improvement and 1 neutral diff.


On Mon, Nov 26, 2018 at 5:08 PM Friedman, Eli <efriedma at codeaurora.org>
wrote:

> On 11/23/2018 12:05 PM, Sanjay Patel via llvm-commits wrote:
> > Author: spatel
> > Date: Fri Nov 23 12:05:12 2018
> > New Revision: 347502
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=347502&view=rev
> > Log:
> > [DAG] consolidate shift simplifications
> >
> > ...and use them to avoid creating obviously undef values as
> > discussed in the post-commit thread for r347478.
>
> This is fine as far as it goes, I guess, but it doesn't really solve
> your problem: replaceAllUsesWith and friends still don't call
> simplifyShift.
>
> -Eli
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181127/a2b58808/attachment.html>