Re: RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
On 5/28/19 4:29 PM, Mikael Vidstedt wrote: > Looks good. Thanks. > The if-define-to-code ratio in LinuxDebuggerLocal.c is impressively high, in > a bad way. An > enhancement to cover cleaning it up sure wouldn’t hurt. I agree. -Aleksey signature.asc Description: OpenPGP digital signature
Re: RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
Looks good. The if-define-to-code ratio in LinuxDebuggerLocal.c is impressively high, in a bad way. An enhancement to cover cleaning it up sure wouldn’t hurt. Cheers, Mikael > On May 28, 2019, at 12:39 AM, Aleksey Shipilev wrote: > > On 5/27/19 12:36 PM, Aleksey Shipilev wrote: >> Bug: >> https://bugs.openjdk.java.net/browse/JDK-8224796 >> >> x86_32 tier1 tests time out (actually, fail) because of this. >> >> In short, recent change to compile C code with --std=c99 got "i386" >> undefined. Build system still >> sets "i586", and we should really use that. I don't want to regress stuff >> much by globally replacing >> i386->i586, so the new code handles *both* defines. This is what "handle >> both x86_64 and amd64" >> block in LinuxDebuggerLocal.c already does. >> >> I have not seen the failures on Mac due to ps_core.c, but it is better to be >> safe there as well. >> >> Fix: >> http://cr.openjdk.java.net/~shade/8224796/webrev.01/ > > David had reviewed. More reviews, please? > > -- > Thanks, > -Aleksey >
Re: RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
On 5/27/19 12:36 PM, Aleksey Shipilev wrote: > Bug: > https://bugs.openjdk.java.net/browse/JDK-8224796 > > x86_32 tier1 tests time out (actually, fail) because of this. > > In short, recent change to compile C code with --std=c99 got "i386" > undefined. Build system still > sets "i586", and we should really use that. I don't want to regress stuff > much by globally replacing > i386->i586, so the new code handles *both* defines. This is what "handle both > x86_64 and amd64" > block in LinuxDebuggerLocal.c already does. > > I have not seen the failures on Mac due to ps_core.c, but it is better to be > safe there as well. > > Fix: > http://cr.openjdk.java.net/~shade/8224796/webrev.01/ David had reviewed. More reviews, please? -- Thanks, -Aleksey signature.asc Description: OpenPGP digital signature
Re: RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
On 28/05/2019 3:05 am, Aleksey Shipilev wrote: On 5/27/19 1:21 PM, David Holmes wrote: I think the existing logic in that file is already quite confused. It should be using defines set by the build system only, to identify CPU etc, not compiler specific defines. So I really don't think we need to keep the i386 stuff in that file as it just adds to the confusion. But I won't fight you on it. Yeah, I was on the fence here too, but don't want to risk more regressions. The "ifs" were rewritten i586 to get the "proper" macro, and the i386 fallback block is just for additional safety. I have not seen the failures on Mac due to ps_core.c, but it is better to be safe there as well. There is no 32-bit macOS build AFAIK. I don't quite understand the comment here: do you suggest to remove that block, or? I am leaning on capturing i586 everywhere i386 was captured before. That code is never built for 32-bit so neither i386 or i586 will ever be defined there. You can make the change but nothing will ever test it. Cheers, David Thanks, -Aleksey
Re: RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
On 5/27/19 1:21 PM, David Holmes wrote: > I think the existing logic in that file is already quite confused. It should > be using defines set by > the build system only, to identify CPU etc, not compiler specific defines. So > I really don't think > we need to keep the i386 stuff in that file as it just adds to the confusion. > But I won't fight you > on it. Yeah, I was on the fence here too, but don't want to risk more regressions. The "ifs" were rewritten i586 to get the "proper" macro, and the i386 fallback block is just for additional safety. >> I have not seen the failures on Mac due to ps_core.c, but it is better to be >> safe there as well. > > There is no 32-bit macOS build AFAIK. I don't quite understand the comment here: do you suggest to remove that block, or? I am leaning on capturing i586 everywhere i386 was captured before. Thanks, -Aleksey signature.asc Description: OpenPGP digital signature
Re: RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
Hi Aleksey,
On 27/05/2019 8:36 pm, Aleksey Shipilev wrote:
Bug:
https://bugs.openjdk.java.net/browse/JDK-8224796
x86_32 tier1 tests time out (actually, fail) because of this.
In short, recent change to compile C code with --std=c99 got "i386" undefined.
Build system still
sets "i586", and we should really use that. I don't want to regress stuff much
by globally replacing
i386->i586, so the new code handles *both* defines. This is what "handle both x86_64
and amd64"
block in LinuxDebuggerLocal.c already does.
I think the existing logic in that file is already quite confused. It
should be using defines set by the build system only, to identify CPU
etc, not compiler specific defines. So I really don't think we need to
keep the i386 stuff in that file as it just adds to the confusion. But I
won't fight you on it.
I have not seen the failures on Mac due to ps_core.c, but it is better to be
safe there as well.
There is no 32-bit macOS build AFAIK.
Thanks,
David
Fix:
http://cr.openjdk.java.net/~shade/8224796/webrev.01/
Testing: {x86_64, x86_32} tier1; jdk-submit (running)
RFR (S) 8224796: C code is not compiled correctly due to undefined "i386"
Bug:
https://bugs.openjdk.java.net/browse/JDK-8224796
x86_32 tier1 tests time out (actually, fail) because of this.
In short, recent change to compile C code with --std=c99 got "i386" undefined.
Build system still
sets "i586", and we should really use that. I don't want to regress stuff much
by globally replacing
i386->i586, so the new code handles *both* defines. This is what "handle both
x86_64 and amd64"
block in LinuxDebuggerLocal.c already does.
I have not seen the failures on Mac due to ps_core.c, but it is better to be
safe there as well.
Fix:
http://cr.openjdk.java.net/~shade/8224796/webrev.01/
Testing: {x86_64, x86_32} tier1; jdk-submit (running)
--
Thanks,
-Aleksey
signature.asc
Description: OpenPGP digital signature
