[PATCH] D59688: [X86] Make post-ra scheduling macrofusion-aware.

Mon Mar 25 14:48:55 PDT 2019

andreadb added a comment.

In D59688#1442004 <https://reviews.llvm.org/D59688#1442004>, @lebedev.ri wrote:

> In D59688#1441464 <https://reviews.llvm.org/D59688#1441464>, @andreadb wrote:
>
> > In D59688#1439408 <https://reviews.llvm.org/D59688#1439408>, @courbet wrote:
> >
> > > > I plan to run some experiments today using your patch.
> > >
> > > That's great, thanks.
> >
> >
> > Sorry, I was over optimistic about my other workload. I don't think I'll get a chance to get any perf numbers anytime soon.
> >
> > That being said, I tried your patch on a few small examples on some different targets, and results seem good.
> >  For example, before your patch I saw cases where the test/cmp was not emitted before the conditional branch. Your patch seems to fix that "issue" in most cases.
> >
> > My only concern is that the macro-fusion mutator might be a bit too aggressive for AMD processors.
> >  X86MacroFusion assumes that branch fusion can happen with ADD/SUB/INC/DEC too. That is okay for Intel processors, but not necessarily for
>
>
>
>
> In D59688#1441464 <https://reviews.llvm.org/D59688#1441464>, @andreadb wrote:
>
> > AMD processors where branch fusion (as far as I remember) is limited to CMP/TEST opcodes only.
>
>
> That is consistent with what is stated in agner's microarchitecture, amd sog for piledriver.
>
> > Since your patch enables that mutator for targets with FeatureMacroFusion, it would be nice to get some feedback from somebody with access to an AMD target where macro fusion is enabled (Bobcat/Jaguar doesn't do branch fusion). Perhaps @lebedev.ri can run some quick tests on BdVer2?
>
> It will, as usual, depend on whether this happens to affect the hotpath or not.
>  I did just run my rawspeed benchmark, and i'm not observing any notable non-noise perf changes.

Thanks. That matches what I also saw in the past when I tested it.

> 
> 
>> I don't think is a blocking issue, but in future we should revisit the logic in X86MacroFusion.
> 
> While there, @andreadb, can you reply on https://reviews.llvm.org/D46662#1293043 ?

I replied to that code review (the two small benchmarks were available from one of Xur’s older posts).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59688