Thread: AW: AW: Could turn on -O2 in AIX
> There is a special case in nabstime.h for AIX, which imho > got swapped. The normal case for me would be INT_MIN > and not the 0x80000001. > There is a comment that I don't understand at all given the below > source code: > > /* > * AIX considers 2147483648 == -2147483648 (since they have > the same bit > * representation) but uses a different sign sense in a comparison to > * these integer constants depending on whether the constant is signed > * or not! > */ > #if defined (_AIX) > #define NOSTART_ABSTIME ((AbsoluteTime) INT_MIN) > #else > #define NOSTART_ABSTIME ((AbsoluteTime) 0x80000001) > /* -2147483647 (- 2^31) */ > #endif > > But that is unfortunately not the problem. Looks like yet > another broken compiler to me :-( Ok, the comparison ((int) time) > ((int) 0x80000001) is the problem. Reading the comment again and again, I have come to the conclusion, that the intent was originally to avoid INT_MIN on AIX. My solution would be to use INT_MIN for all ports, which has the advantage that the above problematic comparison can be converted to !=, since no integer will be smaller than INT_MIN. Please apply the patch and all works well. Thanks Andreas
Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:
>> But that is unfortunately not the problem. Looks like yet
>> another broken compiler to me :-(
> Ok, the comparison ((int) time) > ((int) 0x80000001) is the problem.
> Reading the comment again and again, I have come to the conclusion,
> that the intent was originally to avoid INT_MIN on AIX.
No, I think the other way round. Digging into Postgres 4.2, I find
#if defined(PORTNAME_aix)
/** AIX considers 2147483648 == -2147483648 (since they have the same bit* representation) but uses a different sign
sensein a comparison to * these integer constants depending on whether the constant is signed * or not!*/
#include <values.h>
#define NOSTART_ABSTIME ((AbsoluteTime) HIBITI) /* - 2^31 */
#else
#define NOSTART_ABSTIME ((AbsoluteTime) 2147483648) /* - 2^31 */
#endif /* PORTNAME_aix */
where HIBITI must come from a system header, because it doesn't appear
anywhere else in Postgres 4.2. But I'm betting it was a representation
of 0x80000000. By the time of our oldest CVS sources, this had
metamorphosed into
#if defined(PORTNAME_aix)
/** AIX considers 2147483648 == -2147483648 (since they have the same bit* representation) but uses a different sign
sensein a comparison to * these integer constants depending on whether the constant is signed * or not!*/
#include <values.h>
/*#define NOSTART_ABSTIME ((AbsoluteTime) HIBITI) */ /* - 2^31 */
#define NOSTART_ABSTIME ((AbsoluteTime) INT_MIN)
#else
/*#define NOSTART_ABSTIME ((AbsoluteTime) 2147483648)*/ /* - 2^31 */
#define NOSTART_ABSTIME ((AbsoluteTime) -2147483647) /* - 2^31 */
#endif /* PORTNAME_aix */
Hard to tell how we got from point A to point B, but it seems
crystal-clear that the *original* author intended to use 0x80000000
on all platforms.
> My solution would be to use INT_MIN for all ports, which has the advantage
> that the above problematic comparison can be converted to !=,
> since no integer will be smaller than INT_MIN.
I agree. When I was looking at this code this morning, I was wondering
what INT_MIN was supposed to represent anyway, if NOSTART_ABSTIME is
INT_MIN + 1. I think someone messed this up between 4.2 and Postgres95.
Thomas, any objection to this plan?
regards, tom lane
>> My solution would be to use INT_MIN for all ports, which has the advantage
>> that the above problematic comparison can be converted to !=,
>> since no integer will be smaller than INT_MIN.
> I agree. When I was looking at this code this morning, I was wondering
> what INT_MIN was supposed to represent anyway, if NOSTART_ABSTIME is
> INT_MIN + 1. I think someone messed this up between 4.2 and Postgres95.
> Thomas, any objection to this plan?
I went ahead and committed this change, since Thomas hasn't weighed in
with an objection ...
regards, tom lane
> >> My solution would be to use INT_MIN for all ports, which has the advantage
> >> that the above problematic comparison can be converted to !=,
> >> since no integer will be smaller than INT_MIN.
> > I agree. When I was looking at this code this morning, I was wondering
> > what INT_MIN was supposed to represent anyway, if NOSTART_ABSTIME is
> > INT_MIN + 1. I think someone messed this up between 4.2 and Postgres95.
> > Thomas, any objection to this plan?
> I went ahead and committed this change, since Thomas hasn't weighed in
> with an objection ...
Hmm. I've been seeing a lot of followup messages to threads I don't
recall getting. Presumably all related to a combination of getting
auto-unsubscribed to -hackers :(, traveling and getting mail on my
laptop, and converting service at home over to DSL.
Hopefully this will all settle down here in the next few days.
Is the original issue support for 0x10... as the smallest integer, as
opposed to -MAX_INT? As long as we continue to map the "reserved values"
to the upper and lower range of allowed values so they are unlikely to
appear under normal circumstances, the change should be OK.
- Thomas
Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
> Is the original issue support for 0x10... as the smallest integer, as
> opposed to -MAX_INT? As long as we continue to map the "reserved values"
> to the upper and lower range of allowed values so they are unlikely to
> appear under normal circumstances, the change should be OK.
I think that the original problem was that Andreas was seeing a compiler
codegen bug on AIX, having to do with the comparisonfoo > INT_MIN
generated by the AbsoluteTimeIsReal macro. I think he was seeing that
the compiler insisted on generating an unsigned compare, explicit casts
to signed datatypes notwithstanding :-(.
The proposed fix was to recode the macro's test as foo != INT_MIN,
thereby avoiding the issue of whether the comparison is signed or not.
To do that, we needed to make NOSTART_ABSTIME be defined as INT_MIN
on all platforms, not only AIX. That seemed like a good general-purpose
approach to me anyway, since the intended meaning of 0x80000000 was very
unclear otherwise.
regards, tom lane
PS: I'm quite sure that I'd explicitly cc'd you on the prior discussion.
If you didn't see it, then you've lost personal mail, not only pghackers
traffic...
Tom, can you remind me where we left this? > Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes: > >> But that is unfortunately not the problem. Looks like yet > >> another broken compiler to me :-( > > > Ok, the comparison ((int) time) > ((int) 0x80000001) is the problem. > > Reading the comment again and again, I have come to the conclusion, > > that the intent was originally to avoid INT_MIN on AIX. > > No, I think the other way round. Digging into Postgres 4.2, I find > > #if defined(PORTNAME_aix) > /* > * AIX considers 2147483648 == -2147483648 (since they have the same bit > * representation) but uses a different sign sense in a comparison to > * these integer constants depending on whether the constant is signed > * or not! > */ > #include <values.h> > #define NOSTART_ABSTIME ((AbsoluteTime) HIBITI) /* - 2^31 */ > #else > #define NOSTART_ABSTIME ((AbsoluteTime) 2147483648) /* - 2^31 */ > #endif /* PORTNAME_aix */ > > where HIBITI must come from a system header, because it doesn't appear > anywhere else in Postgres 4.2. But I'm betting it was a representation > of 0x80000000. By the time of our oldest CVS sources, this had > metamorphosed into > > #if defined(PORTNAME_aix) > /* > * AIX considers 2147483648 == -2147483648 (since they have the same bit > * representation) but uses a different sign sense in a comparison to > * these integer constants depending on whether the constant is signed > * or not! > */ > #include <values.h> > /*#define NOSTART_ABSTIME ((AbsoluteTime) HIBITI) */ /* - 2^31 */ > #define NOSTART_ABSTIME ((AbsoluteTime) INT_MIN) > #else > /*#define NOSTART_ABSTIME ((AbsoluteTime) 2147483648)*/ /* - 2^31 */ > #define NOSTART_ABSTIME ((AbsoluteTime) -2147483647) /* - 2^31 */ > #endif /* PORTNAME_aix */ > > Hard to tell how we got from point A to point B, but it seems > crystal-clear that the *original* author intended to use 0x80000000 > on all platforms. > > > My solution would be to use INT_MIN for all ports, which has the advantage > > that the above problematic comparison can be converted to !=, > > since no integer will be smaller than INT_MIN. > > I agree. When I was looking at this code this morning, I was wondering > what INT_MIN was supposed to represent anyway, if NOSTART_ABSTIME is > INT_MIN + 1. I think someone messed this up between 4.2 and Postgres95. > > Thomas, any objection to this plan? > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom, can you remind me where we left this?
It's done ...
regards, tom lane