possible bug in global allocation phase

[Heiko Panther]

Jim Wilson wrote:
> Heiko Panther wrote:
> 
>> TEST_HARD_REG_BIT(used, 32) returns 0 and thus r32 is believed to be
>> available. This happens because "used" is  of type HARD_REG_SET.
>> HARD_REG_SET is a 32 bit integer type, so a test for nonexisting bits
>> will produce wrong results.
> 
> 
> Yes, this looks like a legitimate bug to me.  TEST_HARD_REG_BIT provokes 
> undefined/implementation defined behaviour if given an out-of-range 
> register number.  We can get different behaviour for different targets 
> here.
> 
> I see 3 uses of this idiom in global.c, 5 in reload.c, 5 in reload1.c, 
> and one in local.c.  And that is just a quick check, I haven't tried 
> using grep yet, or trying to verify the count.
> 
> I think we need to fix all such occurences by adding a check against 
> FIRST_PSEUDO_REGISTER.

I applied the fix which I proposed in about 15 places (although a grep 
will yield about 220 occurences) and could then successfully complete my 
failing compilation.

I developed my approach further by adding this to hard-reg-set.h:

/* Create a test with slightly different semantic. Instead of testing
for a bit, we ask if register corresponding to bit is available (bit is
0) . Then, we can return 'not available' for bits we don't have. 
Introduced because some code would test nonexisting bits, get back 0,
and assume the register's available */

#define TEST_HARD_REG_AVAILABLE(SET, BIT) \
(TEST_HARD_REG_BIT_H(~(SET), (BIT)))
#define TEST_HARD_REG_UNAVAILABLE(SET, BIT) \
(!TEST_HARD_REG_BIT_H(~(SET), (BIT)))

The pro is that this approach won't use more cycles than before.

Whatever approach is chosen, lots of source code needs to be examined 
and modified. I can't really spare the time to do that. I see three 
options. What do you think, which one makes most sense?

1. I could send in a patch with the changes that got my stuff working, 
so TEST_HARD_REG_BIT and TEST_HARD_REG_AVAILABLE would be used mixed. 
I'm not sure whether the TEST_HARD_REG_AVALABLE semantic makes sense in 
every place where TEST_HARD_REG_BIT appears. The "old" TEST_HARD_REG_BIT 
macro could be modified so it aborts when the register number is out of 
range. That would cost as many cycles as your approach, worst case.

2. You could implement your approach.

3. I could just create a bug report. Which has its own problems, since 
few people will actually use this host/target combination. And my target 
is not in the main gcc tree.

Regards,
Heiko Panther