Re: [PATCH] Fix one-sized FP arrays returning on MIPS

[Richard Sandiford <rsandifo at redhat dot com>]

Eric Botcazou <ebotcazou@act-europe.fr> writes:
>> Disagree.  See my RFA post for an explanation.  If you still think
>> there's a risk of an ABI change after my explanation there, can you
>> respond to the message saying why?
>
> !         /* Truncate it to the type's mode, or its integer equivalent.
> !            This is subject to TRULY_NOOP_TRUNCATION.  */
>
> Are you sure this could not have any adverse effect on any platform?

As sure as I can be.  

Note that the comment you quote is describing what happened before the
patch when TYPE_MODE is integral.  That in itself isn't new behaviour.

The problem in your testcase, and the problem that the patch should fix,
is simply that we were trying to truncate directly into SFmode rather
than going through SImode.  That's wrong for any target, I think.

Note that "any platform" here means MIPS or xtensa, since they are the
only two ports to use this target hook.  And xtensa has no-op truncations,
so the net effect of:

	  /* Truncate it to the type's mode, or its integer equivalent.
	     This is subject to TRULY_NOOP_TRUNCATION.  */
	  *value = convert_to_mode (int_mode_for_mode (TYPE_MODE (type)),
				    *value, 0);

	  /* Now convert it to the final form.  */
	  *value = gen_lowpart (TYPE_MODE (type), *value);

should simply be to change the mode of *VALUE.

>> But the bug can trigger for C too. Again, see the test case in the RFA
>> post.  So regardless of the Ada situation, your patch is definitely an ABI 
>> change for C.
>
> I again dispute the wording "ABI change".  I think an ABI change happens when 
> two pieces of code around an interface cannot communicate anymore, while 
> they previously could.  Here they cannot from the very beginning.

I disagree.  We'd have a situation in which:

  - You can correctly compile a.c with 3.4.0
  - Compiling b.c happens to trigger an ICE
  - If you upgrade to gcc 3.4.2, compile b.c with it, and link the two
    together, the program won't work, because a.c and b.c would be making
    different assumptions about the return value.

That to me is an ABI change.  ABIs are, after all, a property of
individual object files as well as final executables.

It's especially bad when you consider that the person compiling a.c
might not be the same as the one compiling b.c.  For example, someone
could distribute a library in which a public function has a return type
affected by this bug.  The function itself should compile fine, since
the callee side is OK.  So it's quite feasible that someone would use
3.4.0 to compile this library and distribute it.  And the function in
question would (IMO) adhere to the n32 & n64 ABIs.

Someone trying to compile a call to the affected library function would
hit the ICE.  So suppose we applied your patch.  If the user upgraded to
3.4.2 and tried again, the call would then compile OK, but the library
won't work with it, because 3.4.2 expects the return value to be somewhere
else (the wrong place, IMO).  That sort of difference shouldn't happen
between minor revisions.

>> Less risky?  I strongly disagree that a change to the MIPS ABI code
>> is less risky than the patch I'm proposing, which simply changes the
>> mode of an rtx from SFmode to SImode _after_ the ABI-specific stuff
>> has been dealt with.
>
> I'd tend to think that platform-independent changes are inherently
> more risky that platform-dependent changes, simply because of the
> wider audience.

I take your point that, all other things being equal, a target-independent
change is more risky than a target-dependent one.  But I'm not sure about
the "inherently" bit.  It does depend on a lot on the details, especially
if (as in this case) the two patches do very different things.

Anyway, as I said above, the audience isn't that much wider in this case. ;)
Just two targets rather than one.

> Okay, it appears that we will never reach an agreement here.  You're the 
> maintainer, I'm not, so I think I know who will win. :-)  I'll use your 
> patch for our compiler until it is applied to the 3.4 branch.

Thanks.

> Thanks a lot for (in no particular order) the patch, your help and your 
> patience.

Heh.  Thanks also for your patience, and for pointing out the DFmode bug.

Richard