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

Stepan Dyatkovskiy stpworld at narod.ru
Sun Apr 7 04:51:09 PDT 2013


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
>




More information about the llvm-commits mailing list