On 3/25/19 11:04 AM, Martin Sebor wrote: > On 3/25/19 10:07 AM, Jeff Law wrote: >> On 3/24/19 6:21 PM, Martin Sebor wrote: >>> The error issued when the aligned attribute argument is too big >>> to be represented is incorrect: it says the maximum alignment >>> is 1U << 31 when it should actually be 1 << 28. This was a typo >>> introduced when the error message was enhanced earlier in GCC 9. >>> >>> The test I added to verify the fix for the typo exposed another >>> bug introduced in the same commit as the incorrect value in >>> the error message: assuming that the attribute aligned argument >>> fits in SHWI. >>> >>> The attached patch corrects both problems. It has been tested >>> on x86_64-linux. I will commit it as obvious sometime this week >>> unless there are any objections or suggestions for changes. >>> >>> Martin >>> >>> PS I have a couple of questions related to the affected code: >>> 1) Does GCC support building with compilers where int is not 32 >>> bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is >>> less or more?) >> We've certainly supported 16 bit ints in the past. H8/300 would be an >> example. It defaults to 16 bit ints. But I don't think we've tested >> that in a very long time -- my tester is only testing with -mint32. >> >> Look for INT_TYPE_SIZE in config/*/*.h >> >> We've never supported bootstrapping in that mode, just crosses. > > Thanks, that's what I was after: whether GCC can build natively > with such a compiler where sizeof (int) != 32. Sounds like it > can't, i.e., HOST_BITS_PER_INT is always at least 32. Or do > you suppose it's always exactly 32? At least 32 for HOST_BITS_PER_INT, I think we've supported 64 HOST_BITS_PER_INT as well.
HOST_BITS_PER_INT is defined indirectly via CHAR_BIT * SIZEOF_INT, both of which are determined during the configure phase. > >> >> AVR is probably the most interesting as it even has an flag to make >> "int" be 8 bits. It's probably the best tested target in this space. >> >> BITS_PER_UNIT is more of a hardware characteristic. It's generally 8. >> THough I thought one of the TI chips defined it to 32. I suspect you >> weren't really looking for BITS_PER_UNIT here. > > I think using BITS_PER_UNIT here is actually another bug in > the function: it should be using CHAR_BITS instead, like so: > > if (log2align >= HOST_BITS_PER_INT - exact_log2 (CHAR_BITS)) > { > error ("requested alignment %qE exceeds maximum %u", > align, 1U << (HOST_BITS_PER_INT - exact_log2 (CHAR_BITS) - 1)); > return -1; > } Isn't CHAR_BIT a function of the host, not the target? In which case I don't think it's right since supported alignments are primarily a target property. > > Thanks. I was concerned about the test I added breaking on > systems that don't define __INT64_TYPE__, but I see other tests > that assume that __INT64_TYPE__ exists, so if it does break, it > won't be the only one. I wouldn't stress too much about it. The AVR guys might come back with a patch to adjust the test. But again, I wouldn't worry about it until they do. jeff