[PATCH] D19138: [X86] Enable the post-RA-scheduler for 32-bit cpus

[Mitch Bodart via llvm-commits]

mbodart added inline comments.

================
Comment at: lib/Target/X86/X86.td:290
@@ +289,3 @@
+
+def : ProcessorModel<"pentium-m", GenericPostRAModel,
+                     [FeatureX87, FeatureSlowUAMem16, FeatureMMX,
----------------
aaboud wrote:
> Can you create a class for GenereicPostRAModel, similar to class Proc, something like this:
> 
> class ProcPostRA<string Name, list<SubtargetFeature> Features>
>  : ProcessorModel<Name, GenereicPostRAModel, Features>;
I think that might add more confusion than clarity.

There are currently two base classes used to define the cpus, Proc and ProcessorModel.
ProcessorModel is used in cases where we want to override the scheduling model.
Post-RA scheduling is just one property of the scheduling model.

In the cases where we do create a new class derived from ProcessorModel, it is done to encapsulate the processor "features" in one place.

So my preference is to keep the current approach, though it's a minor enough detail
that I can change it if others feel strongly.

================
Comment at: test/CodeGen/X86/pr16360.ll:2
@@ -1,3 +1,3 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mcpu=pentium4 -mtriple=i686-pc-linux | FileCheck %s
 
----------------
aaboud wrote:
> Why did not you simply add the "-post-RA-scheduler=false" flag like in other tests?
As a general rule I would think we want to avoid adding special options to tests,
as that can sometimes mask issues.  But there are some exceptions.

I disabled the post-RA-scheduler in misched-ilp.ll because that test is looking for
a specific behavior from an earlier scheduling phase.

As for machine-cp.ll, I could go either way (updating the test or disabling the post scheduler).
The test is built with the x86_64 triple and nocona cpu.  If built by clang with an x86_64 triple,
the post scheduler would not be enabled as the cpu would be set to "x86_64", not nocona.
So I arbitrarily chose to match this behavior.  But I could simply update the test as well.
Is there any precedence or BKM here?


http://reviews.llvm.org/D19138