[llvm-commits] [llvm] r127518 - in /llvm/trunk: lib/Target/ARM/ARMFastISel.cpp test/CodeGen/ARM/fast-isel-pred.ll

[Eric Christopher]

>> 
>> I make no claims about how well fast-isel works for darwin targets so it's turned off there - so this testcase isn't valid for non-darwin at the moment.
> 
> Ah, right. Forgot it was turned off entirely for non-darwin. Just putting the info on the RUN line should take care of it, then.
> 
>> 
>>> This was code-gen OK before, but failed when trying to assemble due to an invalid instruction (predicate on a non-predicable instruction), right? If so, this test won't catch that as it doesn't try to assemble the assembly file. Adding a CHECK-NOT for the particular instruction that was bogus and a CHECK for what it actually should be would be good.
>> 
>> This would assert with a Debug+Asserts build.
> 
> It'd still be good for the -Asserts build to be able to detect the failure, though, right? The CHECK/CHECK-NOT also help self-document what was going wrong.
> 

That's fair, and probably useful too. :)

>>> Not a huge deal, but seems a bit largish for this sort of test. Does it reduce down at all? I would, perhaps naively, expect that a conditional followed by the vcvt (sitofp <4 x i32> %1 to <4 x float>) would do the trick.
>> 
>> I believed the original submitter when she said she couldn't get it any smaller :)
> 
> As we talked about offline, we can probably get a bit smaller, but not too much. Fast-isel heuristics are probably playing some games here. IMO, worth trying a bit, but not spending a huge amount of time on it. Maybe just reduce to a single function and add an explanatory comment about what's really being checked for?

I'll see what I can do :)

-eric