[PATCH] D37055: [ARM] Reverse PostRASched subtarget feature logic

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

Thanks for looking into this again. This is indeed better. Did you choose the "negative" DisablePostRAScheduler feature over a "positive" FeaturePostRAScheduler just so it's less lines needed in the .td file? The positive variant seems slightly user friendlier and in line with aarch64 to me, even though it means adding the flag to every processor variant except for swift and cyclone.

Either way this is an improvement and should go in, LGTM.



================
Comment at: lib/Target/ARM/ARMSubtarget.cpp:364
     return false;
-  return (!isThumb() || hasThumb2());
+  return !isThumb1Only();
 }
----------------
samparker wrote:
> javed.absar wrote:
> > samparker wrote:
> > > javed.absar wrote:
> > > > may be add an explicit comment on why we choose to do this
> > > 'Because the gods of old said so.' Honestly, I'm really not sure of the reason and the logic seems odd to me. Unfortunately a lot of tests rely on this behaviour and its not something that I want to look into at the moment.
> > I think it is because of IT block - we don't want PostRA to mess it up.
> Ah, that makes sense, thanks!
I don't know if that is the original reason / motivation for it. But indeed the machine scheduler was not prepared/had bugs when dealing with instruction bundles last time I looked and so would probably fail on IT blocks.


https://reviews.llvm.org/D37055