On Tue, Dec 03, 2019 at 01:11:55PM -0500, Michael Meissner wrote:
> On Fri, Nov 22, 2019 at 08:11:16PM -0600, Segher Boessenkool wrote:
> > On Thu, Nov 14, 2019 at 05:56:50PM -0500, Michael Meissner wrote:
> > > * lib/target-supports.exp
> > > (check_effective_target_powerpc_future_ok): Do not require 64-bit
> > > or Linux support before doing the test. Use a 32-bit constant in
> > > PLI.
> >
> > You changed from 0x12345 to 0x1234, instead -- why?
>
> The last time I did the patch there was discussion about 32-bit support. I
> just rewrote the patch to accomidate possibly using -mcpu=future on a 32-bit
> platform. In particular, AIX. So I rewrote the basic test so that in theory
> it would run on a 32-bit platform.
I don't see how this has anything to with it? The insns was just fine
for 32-bit already?
> > > +# Return 1 if this is a PowerPC target supporting -mcpu=future.
> > > proc check_effective_target_powerpc_future_ok { } {
> > > - if { ([istarget powerpc64*-*-linux*]) } {
> > > + if { ([istarget powerpc*-*-*]) } {
> > > return [check_no_compiler_messages powerpc_future_ok object {
> > > int main (void) {
> > > long e;
> > > - asm ("pli %0,%1" : "=r" (e) : "n" (0x12345));
> > > + asm ("pli %0,%1" : "=r" (e) : "n" (0x1234));
> > > return e;
> > > }
> > > } "-mfuture"]
> >
> > You are still passing -mfuture.
>
> All of the tests use the -m<flag> test and not -mcpu=<cpu> test
> (i.e. -mpower9-vector instead of -mcpu=power9 or -mdejagnu-cpu=power9), so I
> was being consistant with those tests.
Yes, that is a nasty problem, so let's not promulgate that. Anyway, it
also contradicts the comment on this function.
> > I don't understand why we test both pli and pld.
>
> Just being cautious.
Hrm, the check_effective_target_powerpc_future_ok test should probably
use a non-prefixed instruction? If we cannot do that right now, add a
comment?
> > > +proc check_effective_target_powerpc_prefixed_addr_ok { } {
> >
> > The name says another.
> >
> > > + if { ([istarget powerpc*-*-*]) } {
> >
> > This part has no function? Are there any testcases that test for our
> > prefixed insns that are compiler for non-powerpc?
>
> Probably not, but all of the other tests have the same prefix.
Feel free to send patches fixing old mistakes, too.
> > > + return [check_no_compiler_messages powerpc_prefixed_addr_ok object {
> > Does this need to be separate from check_effective_target_powerpc_future_ok
> > at all?
>
> This gets back to whether some port will eventually want to use FUTURE
> instructions in a 32-bit context. Linux will always be 64-bit little endian,
No, Linux also supports 32-bit BE and 64-bit BE. Even if we do not
support Future on those currently, this may change. There is absolutely
no reason to make it harder to do later. If we think some combinations
do not exist we can *ignore* them. That is much less work, too.
> but there are other platforms out there that may want to generate FUTURE code.
But we should not anticipate those having broken assemblers that do not
support half of the insns, etc. You *already* required prefixed insns in
the base Future test, to prove how futile this is.
> > > +# support PC-relative addressing when -mcpu=future is used to pass this
> > > test.
> > > +
> > > +proc check_effective_target_powerpc_pcrel_ok { } {
> > > + if { ([istarget powerpc*-*-*]) } {
> > > + return [check_no_compiler_messages powerpc_pcrel_ok object {
> > > + int main (void) {
> > > + static int s __attribute__((__used__));
> > > + int e;
> > > + asm ("plwa %0,s@pcrel(0),1" : "=r" (e));
> > > + return e;
> > > + }
> > > + } "-mfuture"]
> > > + } else {
> > > + return 0
> > > + }
> > > +}
> >
> > So every assembler will support either all three of these, or none.
> > Can you simplify this please?
>
> I can imagine for example AIX might support 64-bit 'future' but not support
> PC-relative. I believe you (or David) asked to split up the prefixed
> addressing with numeric offets and PC-relative addressing, because ports might
> not be able to support PC-relative addressing.
That is fine, but these tests do not do that. Some clearer comments will
help, too.
Segher
