Differential D10787
Expose the ILP32/LP64 programming environments based on __ILP32__/__LP64__ instead of by architecture. Authored by ngie on May 18 2017, 6:54 AM. Tags None Referenced Files
Subscribers
Details Expose the ILP32/LP64 programming environments based on __ILP32__/__LP64__ instead of by architecture. The list was incomplete (previous commits purged invalid architectures, like __alpha__, but failed to add new ones). It's best to base the symbol presence on whether or not the architecture is ILP32 / LP64 capable, per the compiler. This fixes the ILP32/LP64 program environments on arm64. MFC after: 1 month Ran kyua tests on amd64 and i386 without issue.
Diff Detail
Event TimelineComment Actions Actually I think I take that back... mips and arm aren't listed at all. Are they affected? Comment Actions Only arm64 exposes __ILP32__ and __LP64__ (depending on whether or not it's 32-bit or 64-bit), per a quick grep of contrib/gcc and contrib/llvm . Comment Actions All the architectures do. We use it elsewhere in the tree and it works. Your grep has lead you astray. contrib/gcc/config/i386/freebsd64.h: builtin_define ("LP64"); \ are just three examples. What I'm wondering about thought is why aren't arm and mips in this list at all? And why does powerpc assume 32 bit? Comment Actions I meant to say "only arm64 exposes it, in addition to the other architectures currently handled".
Beats me why arm/mips were never considered, but there is an issue potentially with how __ILP32__, __LP32__, and __LP64__ are defined on those architectures. This review specifically addresses the hardcoded have_<FOO> for LP64 and ILP32 that was incorrect (maybe not when this was first written, but wasn't correct after additional architectures were added). What ILP32/LP32/LP64 are is a different topic discussed more at length here: http://www.unix.org/whitepapers/64bit.html (this is what I was looking for -- it helps because I personally didn't understand the nuances between ILP32 and LP32, until I read the whitepaper). Comment Actions Correct, and my point, which I've not yet gotten an answer to, is that it pulls in several new architectures into defining have_ILP32_OFFBIG (adding 32-bit mips, risc V, and 32-bit arm) and have_LP64_OFF64 (adding arm64 and mips64). Why did they work before? Or do you need to make adjustments elsewhere to get them to build now? Have they been tested?
Yea, we have no LP32 architectures, so you can ignore that. All our archs have 32-bit ints. Comment Actions I don't genuinely know if they ever worked before. This functionality is somewhat obscure -- I ran into it purely because I wanted to add some tests for this program (in this case to exercise the -v flag), and in order to do so, I needed to know what was available (because unfortunately the documentation was very lacking). I ran into this and grew confused with the hardcoded list of architectures, because it seemed that this functionality had indeed been broken for some architectures that should support it. Comment Actions ...
Hi Warner: do you have any valid concerns about this change, or is it ok if I commit this? Comment Actions ...
I'm going to commit this change in 3 days if I don't hear anything from the reviewers. |