Hi! On 12/13/21 10:54 AM, Segher Boessenkool wrote: > On Mon, Dec 13, 2021 at 11:30:28AM -0500, David Edelsohn wrote: >> On Mon, Dec 13, 2021 at 10:48 AM Bill Schmidt <wschm...@linux.ibm.com> wrote: >>> Hi! >>> >>> PR103624 observes that we get segfaults for the 64-bit darn builtins when >>> compiled >>> on a 32-bit architecture. The old built-in infrastructure requires >>> TARGET_64BIT, and >>> this was missed in the new support. Moving these two builtins from the >>> [power9] >>> stanza to the [power9-64] stanza solves the problem. >>> >>> Tested the fix on a powerpc-e300c3-linux-gnu cross. Bootstrapped and >>> tested on >>> powerpc64le-linux-gnu with no regressions. Is this okay for trunk? >> Okay. > No, as I said before this is not correct, not without a lot more > explanation at least. We should not copy errors in the old code into > the new code. That is negating one of the main advantages of > reimplementing this in the first place!
Can you please be more specific? All I have from you before is "It should work for 32-bit though?" I responded in the bug report that __builtin_darn_32 was used for this purpose. I haven't seen a response to that. What do you want to see happen? The patterns in rs6000.md are darn_32, gated by TARGET_P9_MISC; darn_raw, gated by TARGET_P9_MISC && TARGET_64BIT; and darn, gated by TARGET_P9_MISC && TARGET_64BIT. The builtins correspond to these patterns in the obvious way. If you think that these patterns should be enabled differently, that's fine, but that's a completely different patch than fixing the incorrect built-ins to match what the patterns do and thus avoid ICEing. Thanks, Bill > > Segher > > >>> PR target/103624 >>> * config/rs6000/rs6000-builtin-new.def (__builtin_darn): Move to >>> [power9-64] stanza. >>> (__builtin_darn_raw): Likewise.
