On Tue, Dec 14, 2021 at 07:32:30AM -0600, Bill Schmidt wrote: > On 12/13/21 6:22 PM, Segher Boessenkool wrote: > > On Mon, Dec 13, 2021 at 02:37:43PM -0600, Bill Schmidt wrote: > >> 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: > >>>>> 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? > > That of course does not work for _raw. > > > > These builtins should just return a "long", just like __builtin_ppc_mftb > > does. All three of them. > > Well, that seems wrong for __builtin_darn_32, which maps to an SImode pattern.
That is Yet Another Bug, then. The insn returns a full register. The patterns should use either :P or :GPR (the latter if SImode makes sense for it, so we could have that for all darn variants). :DI and :SI never make sense for this. > So, I assume what you'd like to see is for the other two built-ins to return > long, and for the "&& TARGET_64BIT" to be removed from the darn_raw and darn > patterns? No, all builtins should work in either mode, and always return long. If the patterns are broken, the *patterns* should be fixed :-) > > Avoiding ICEs should not be a goal. It should be a side effect of doing > > the right thing in the first place! > > There's no reason to get snippy. Given that you approved Kelvin's original > implementation of the darn patterns and built-in functions, I think I can be > forgiven for thinking that those were the desired semantics. :-) Sorry if I sound annoyed. I am annoyed, but not with you. Just with the world in general I suppose. With the new builtins representation it is much easier to spot problems, it is a great success already! Segher
