[llvm] r178854 - Buildbot fix for r178851: mistake was in wrong TargetRegisterInfo::getRegClass usage.

[Renato Golin]

Hi Stepan,

You are right, the test remained broken until I cleaned the directory.
However, after Jakob's enquires, I think we should still keep it off until
another round of review is done. Shouldn't be a problem, since the patch is
localized and doesn't depend on APIs that change too often.

If Jacob is happy with your patch, I'm fine with it as long as it passes at
least check-all and test-suite on an A8 or higher and on x86_64.

cheers,
--renato


On 7 April 2013 12:51, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:

> Hi,
> Eh.. guys, I looked at clang-native-arm-lnt after my patch was reverted,
> Revert of r178854 didn't fix any of failed tests, revision number of
> revert commit is 178883.  In #452 build (r178904) it is still failed (few
> hours after Renato reverted my commits). Actually it have been fixed
> somewhere between 178904 and 178923).
>
> But I'm still very sorry about ugly r178851 and *absent* reaction after
> 178854. Currently my beagleboard rebuilds everything, so I'll publish test
> reports soon.
>
> I also have question about TargetRegisterInfo::**getMinimalPhysRegClass
> and QPRRegClass.contains(). My purpose in this code is to find the minimal
> class for register. I supposed this method does exactly what I need.
> So do you propose to replace it with SPRRegClass/DPRRegClass/**QPRRegClass.contains
> methods set?
>
> -Stepan.
>
>
> Jakob Stoklund Olesen wrote:
>
>>
>> On Apr 5, 2013, at 11:36 AM, Renato Golin <renato.golin at linaro.org>
>> wrote:
>>
>>  On 5 April 2013 19:26, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>>>
>>> That's not right either. How are you testing this code? Please don't
>>> just guess.
>>>
>>> You should probably be using QPRRegClass.contains().
>>>
>>> Hi Jacob,
>>>
>>> I have reverted this (and the original) patch already. This is not right
>>> and there isn't enough tests to make me sleep at night.
>>>
>>> Unless I see clear evidence that a patch in that area is actually doing
>>> what it should (via review of someone that knows it, like you) AND proper
>>> tests are passing (including the test-suite on ARM), it'll remain reverted.
>>>
>>
>> Thanks, Renato.
>>
>> I agree.
>>
>> /jakob
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130407/6c9ab87b/attachment.html>