[compiler-rt] fix gcc 12 support by removing enum-type-specifier

[yuxuanchen1997]

This unsigned int here is a new addition introduced in #164713 and doesn't compile under older (<13) gcc (see godbolt), which LLVM officially supports from 7.4+. This change should be fine as none of the enum values are negative.

[compnerd]

This isn't fine sadly - the signedness is platform specific. This needs to be preserved. I think that requiring GCC 13+ is acceptable. This is not building LLVM, it is target code.

[yuxuanchen1997]

This isn't fine sadly - the signedness is platform specific. This needs to be preserved. I think that requiring GCC 7+ is acceptable.

Sorry if I didn't make it clear. I am trying to say that the enum doesn't need to be unsigned int. It used to be just int before #164713. This code used enum-type-specifier which is going to push GCC requirement to 13+ from the current 7+.

[compnerd]

This isn't fine sadly - the signedness is platform specific. This needs to be preserved. I think that requiring GCC 7+ is acceptable.

Sorry if I didn't make it clear. I am trying to say that the enum doesn't need to be unsigned int. It used to be just int before #164713. This code used enum-type-specifier which is going to push GCC requirement to 13+ from the current 7+.

But it does. The recent change changed the struct member from unsigned int to the enumerator type. If you want to make this ABI compliant, I think that we should revert the change to the struct to use the unsigned int.

[mikolaj-pirog]

Thanks @yuxuanchen1997 for pointing out that using typed enums is c23, I missed that.

My goal with introducing the enum was to prevent errors like the one I fixed in the PR with CWF. I think we could keep the enum as an argument type, revert to unsigned int in the struct, and cast when calling the function. Would that be okay?

[yuxuanchen1997]

If you want to make this ABI compliant, I think that we should revert the change to the struct to use the unsigned int.

Would be in favor of that solution.

[yuxuanchen1997]

Thanks @yuxuanchen1997 for pointing out that using typed enums is c23, I missed that.

My goal with introducing the enum was to prevent errors like the one I fixed in the PR with CWF. I think we could keep the enum as an argument type, revert to unsigned int in the struct, and cast when calling the function. Would that be okay?

Thanks. That would be ok for me.

[compnerd]

Thanks @yuxuanchen1997 for pointing out that using typed enums is c23, I missed that.

My goal with introducing the enum was to prevent errors like the one I fixed in the PR with CWF. I think we could keep the enum as an argument type, revert to unsigned int in the struct, and cast when calling the function. Would that be okay?

Yeah, that would be fine. I don't have a strong opinion between the two as long as the ABI is not changed.

[mikolaj-pirog]

Alright, I can file a PR with a fix shortly

[yuxuanchen1997]

Thanks @yuxuanchen1997 for pointing out that using typed enums is c23, I missed that.

My goal with introducing the enum was to prevent errors like the one I fixed in the PR with CWF. I think we could keep the enum as an argument type, revert to unsigned int in the struct, and cast when calling the function. Would that be okay?

Double checking this... You probably still have (some theoretical) ABI issues if the enums are used as arguments to getIntelProcessorTypeAndSubtype and getAMDProcessorTypeAndSubtype. Having pointers to signed enum as arguments, the two functions will do a signed to signed assignment by treating the argument pointer as one to a signed int, where the caller has passed one to an unsigned int. I think ideally you'd want to preserve that signed to unsigned assignment if you want to be able to pass &(__cpu_model.__cpu_type).

[compnerd]

Thanks @yuxuanchen1997 for pointing out that using typed enums is c23, I missed that.
My goal with introducing the enum was to prevent errors like the one I fixed in the PR with CWF. I think we could keep the enum as an argument type, revert to unsigned int in the struct, and cast when calling the function. Would that be okay?

Double checking this... You probably still have (some theoretical) ABI issues if the enums are used as arguments to getIntelProcessorTypeAndSubtype and getAMDProcessorTypeAndSubtype. Having pointers to signed enum as arguments, the two functions will do a signed to signed assignment by treating the argument pointer as one to a signed int, where the caller has passed one to an unsigned int. I think ideally you'd want to preserve that signed to unsigned assignment if you want to be able to pass &(__cpu_model.__cpu_type).

Correct, but the members of the structure is more what I was concerned about. The other functions should be fine (they shouldn't be used outside of the module).

[mikolaj-pirog]

Here's a PR #165048