[llvm] r233153 - 'optnone' should not disable DAG combiner.

[David Blaikie]

On Wed, Mar 25, 2015 at 8:46 AM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

>  Leave the test case in and switch the expectation (& perhaps add a
> comment explaining why this the right choice and the previous one wasn't)
> so there's breadcrumbs if anyone else gets the same idea again in the
> future?
>
> Hmmmm the conditions where the test failed were pretty obscure and we used
> a sledgehammer to drive a finishing nail. But I could see a way to make the
> test prove that if an optnone function bails out of fast-isel, the DAG
> combiner does run, which would be the point you're driving at.  It'd get a
> false failure if fast-isel got smarter but with the right commentary that
> ought to be okay.  Will work on this later today.
>

To keep the test non-brittle (though perhaps it'd become pointless, rather
than a false failure) you could phrase the test in terms of comparing
optnone and non-optnone behavior, ensuring they produce identical answers?
Maybe - perhaps that's not a good idea, I'm not sure.

- David


>  --paulr
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Tuesday, March 24, 2015 7:13 PM
> *To:* Robinson, Paul
> *Cc:* llvm-commits at cs.uiuc.edu
> *Subject:* Re: [llvm] r233153 - 'optnone' should not disable DAG combiner.
>
>
>
>
> On Mar 24, 2015 5:18 PM, "Paul Robinson" <
> paul_robinson at playstation.sony.com> wrote:
> >
> > Author: probinson
> > Date: Tue Mar 24 19:10:24 2015
> > New Revision: 233153
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=233153&view=rev
> > Log:
> > 'optnone' should not disable DAG combiner.
> >
> > Reverts the code change from r221168 and the relevant test.
> > It was a mistake to disable the combiner, and based on the ultimate
> > definition of 'optnone' we shouldn't have considered the test case
> > as failing in the first place.
>
> Leave the test case in and switch the expectation (& perhaps add a comment
> explaining why this the right choice and the previous one wasn't) so
> there's breadcrumbs if anyone else gets the same idea again in the future?
>
> >
> > Removed:
> >     llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll
> > Modified:
> >     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=233153&r1=233152&r2=233153&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Mar 24
> 19:10:24 2015
> > @@ -1183,11 +1183,6 @@ void DAGCombiner::Run(CombineLevel AtLev
> >    LegalOperations = Level >= AfterLegalizeVectorOps;
> >    LegalTypes = Level >= AfterLegalizeTypes;
> >
> > -  // Early exit if this basic block is in an optnone function.
> > -  if (DAG.getMachineFunction().getFunction()->hasFnAttribute(
> > -          Attribute::OptimizeNone))
> > -    return;
> > -
> >    // Add all the dag nodes to the worklist.
> >    for (SelectionDAG::allnodes_iterator I = DAG.allnodes_begin(),
> >         E = DAG.allnodes_end(); I != E; ++I)
> >
> > Removed: llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll?rev=233152&view=auto
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/fastmath-optnone.ll (removed)
> > @@ -1,35 +0,0 @@
> > -; RUN: llc < %s -mcpu=corei7 -march=x86-64 -mattr=+sse2 | FileCheck %s
> > -; Verify that floating-point operations inside 'optnone' functions
> > -; are not optimized even if unsafe-fp-math is set.
> > -
> > -define float @foo(float %x) #0 {
> > -entry:
> > -  %add = fadd fast float %x, %x
> > -  %add1 = fadd fast float %add, %x
> > -  ret float %add1
> > -}
> > -
> > -; CHECK-LABEL: @foo
> > -; CHECK-NOT: add
> > -; CHECK: mul
> > -; CHECK-NOT: add
> > -; CHECK: ret
> > -
> > -define float @fooWithOptnone(float %x) #1 {
> > -entry:
> > -  %add = fadd fast float %x, %x
> > -  %add1 = fadd fast float %add, %x
> > -  ret float %add1
> > -}
> > -
> > -; CHECK-LABEL: @fooWithOptnone
> > -; CHECK-NOT: mul
> > -; CHECK: add
> > -; CHECK-NOT: mul
> > -; CHECK: add
> > -; CHECK-NOT: mul
> > -; CHECK: ret
> > -
> > -
> > -attributes #0 = { "unsafe-fp-math"="true" }
> > -attributes #1 = { noinline optnone "unsafe-fp-math"="true" }
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150325/5d68610b/attachment.html>