llvm dev - Dec 2016 - AtomicExpandPass and branch weighting

I'm working on a change to the layout algorithm, and I noted that
test/CodeGen/ARM/cmpxchg-weak.ll was affected.

Normally, that would be fine, but I noted that the layout changed the
fallthrough from the success case to the failure case. I was surprised to
see that the success case isn't annotated with a branch weight by
AtomicExpandPass.cpp

Would it make sense to annotate the success case as more likely when we
expand the intrinsic to help guarantee that the success case remains the
fallthrough? Even a 2:1 or 3:2 weighting would correct the layout issue I
noted.

Thanks,
Kyle.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161212/5b056dc5/attachment.html>

----- Original Message -----
> From: "Kyle Butt via llvm-dev" <llvm-dev at lists.llvm.org>
> To: "Tim Northover" <t.p.northover at gmail.com>
> Cc: "LLVM Developers" <llvm-dev at lists.llvm.org>
> Sent: Monday, December 12, 2016 11:30:32 AM
> Subject: [llvm-dev] AtomicExpandPass and branch weighting
> I'm working on a change to the layout algorithm, and I noted that
> test/CodeGen/ARM/cmpxchg-weak.ll was affected.
> Normally, that would be fine, but I noted that the layout changed the
> fallthrough from the success case to the failure case. I was
> surprised to see that the success case isn't annotated with a branch
> weight by AtomicExpandPass.cpp
> Would it make sense to annotate the success case as more likely when
> we expand the intrinsic to help guarantee that the success case
> remains the fallthrough?
Certainly makes sense to me. 

-Hal 
> Even a 2:1 or 3:2 weighting would correct the layout issue I noted.
> Thanks,
> Kyle.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 

Hal Finkel 
Lead, Compiler Technology and Programming Languages 
Leadership Computing Facility 
Argonne National Laboratory 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161214/5ae43f8b/attachment.html>

Seems reasonable.

I'd note additionally that on some architectures, that the success block
*must* be the fallthrough case (that is to say: you must not have any taken
branches between the load-linked and store-conditional) in order to have an
architectural guarantee that two such loops on different CPUs won't livelock
against eachother.


On Dec 12, 2016, at 12:30 PM, Kyle Butt via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> I'm working on a change to the layout algorithm, and I noted that
test/CodeGen/ARM/cmpxchg-weak.ll was affected.
> 
> Normally, that would be fine, but I noted that the layout changed the
fallthrough from the success case to the failure case. I was surprised to see
that the success case isn't annotated with a branch weight by
AtomicExpandPass.cpp
> 
> Would it make sense to annotate the success case as more likely when we
expand the intrinsic to help guarantee that the success case remains the
fallthrough? Even a 2:1 or 3:2 weighting would correct the layout issue I noted.
> 
> Thanks,
> Kyle.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Wed, Dec 14, 2016 at 1:34 PM, James Knight <jyknight at google.com>
wrote:
> Seems reasonable.
>
> I'd note additionally that on some architectures, that the success
block
> *must* be the fallthrough case (that is to say: you must not have any taken
> branches between the load-linked and store-conditional) in order to have an
> architectural guarantee that two such loops on different CPUs won't
> livelock against eachother.
>
Do we have a way to *require* that 2 blocks be laid out consecutively? I
don't think that we do. A hint is better than nothing, but not a guarantee.

>
>
> On Dec 12, 2016, at 12:30 PM, Kyle Butt via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> > I'm working on a change to the layout algorithm, and I noted that
> test/CodeGen/ARM/cmpxchg-weak.ll was affected.
> >
> > Normally, that would be fine, but I noted that the layout changed the
> fallthrough from the success case to the failure case. I was surprised to
> see that the success case isn't annotated with a branch weight by
> AtomicExpandPass.cpp
> >
> > Would it make sense to annotate the success case as more likely when
we
> expand the intrinsic to help guarantee that the success case remains the
> fallthrough? Even a 2:1 or 3:2 weighting would correct the layout issue I
> noted.
> >
> > Thanks,
> > Kyle.
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > http://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/20161214/d98951a6/attachment.html>