Re: [PATCH, ARM] stop changing signedness in PROMOTE_MODE

[Jim Wilson <jim.wilson@linaro.org>]

On Wed, Jul 15, 2015 at 6:04 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Tue, 14 Jul 2015, Jim Wilson wrote:
>
>> Now that we do have the problem, we can't fix it without an ARM port ABI
>> change, which is undesirable, so we may have to fix it with a MI change.
>
> What's the ABI implication of fixing the inconsistency?

Currently signed chars and signed shorts are passed sign-extended.  If
we make TARGET_PROMOTE_FUNCTION_MODE work the same as PROMOTE_MODE,
then they will be passed zero-extended.

Given the testcase:

int sub (int) __attribute__ ((noinline));
int sub2 (signed char) __attribute__ ((noinline));
int sub (int i) { return sub2 (i); }
int sub2 (signed char c) { return c & 0xff; }

Currently sub will do a char sign-extend to convert the int to signed
char, and sub2 will do a char zero-extend for the and.  With the
change, sub will do a char zero-extend to convert the int to unsigned
char, and sub2 will do nothing.  If you compile sub without the change
and sub2 with the change, then you lose the and operation and get a
sign-extended char at the end.

>> There were two MI changes suggested, one was fixing the out-of-ssa pass
>> to handle SUBREG_PROMOTED_P promotions.  The other was to disallow
>> creating PHI nodes between parms and locals.  I haven't had a chance to
>> try implementing the second one yet; I hope to work on that today.
>
> Don't bother with the latter, it doesn't have a chance of being accepted.

I tried looking at it anyways, as I need to learn more about this
stuff.  It didn't seem feasible without changing a lot of optimization
passes which doesn't seem reasonable.

> If the terrible hack in outof-ssa really will be necessary (and I really
> really hope it won't) then I think I prefer the approach you partly tried
> in comment #12 of PR 65932 already.  Let partition_to_pseudo[] refer to
> the promoted subreg and deal with that situation in emit_partition_copy;
> I'd then hope that the unsignedsrcp parameter could go away (unfortunately
> the sizeexp will have to stay).

Yes, I think that is a cleaner way to do it, but I had trouble getting
that to work as I don't know enough about the code yet.  Doing it
directly in emit_partition_copy was easier, just to prove it could
work.  I can go back and try to make this work again.

Jim