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

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
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
 
----------------
mbodart wrote:
> 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?
I don't think this is actually written anywhere as an official guideline, but yes, I think we should prefer to only use special options when they are supposed to affect the outcome of the test. That gives us better test coverage for the common case.

There's a huge pile of x86 tests that excessively specify a CPU model when they really should specify an attribute (for example as seen in this patch, -mcpu=pentium4 rather than -mattr=sse2). We should try to fix those as we encounter them. 

So my vote would be to update all of the affected test files with the flags that they are really testing (these would be test-file-only commits ahead of this one). It will make this patch smaller and the intended effect of this patch will be made clear rather than confused by a bunch of unintentional changes because test files were poorly specified.


http://reviews.llvm.org/D19138