[RFC] PR 58542: const_int vs lost modes

In this pr, we have a -1 in type __int128.  Since this value can be
represented in a HOST_WIDE_INT, we expand this to a const_int.

The expansion from tree to rtl happens in expand_builtin_atomic_store.  And as
with most of our builtins, we then pass off the rtl to another routine for
expansion.  When we fill in the blanks of the expand_operand in
expand_atomic_compare_and_swap, we've forgotten that the const_int is of
TImode.  Then convert_modes attempts to zero-extend what it assumes is a
narrower mode and we get corrupt data.

For a bit I thought that the bug was in convert_modes, but honestly any
change I make there merely moves the bug around.  The biggest problem is
that we've lost the mode.

Given that we lose the mode through any of 10 stack frames, I believe it
to be implausible to adjust all of the call frames in optabs.c.  The real
bug is that const_int doesn't carry a mode.

The most isolated patch I can come up with, especially since we ought to
fix this in 4.8 branch as well, is to only allow expansion of wide int
modes to const_int when they're positive.

Thoughts?


r~
PR rtl/58542
	* emit-rtl.c (immed_double_const): Use const_double for negative
	numbers of very wide modes.

> The most isolated patch I can come up with, especially since we ought to
> fix this in 4.8 branch as well, is to only allow expansion of wide int
> modes to const_int when they're positive.
> 
> Thoughts?

That's a fairly dangerous hack in my opinion, in particular this breaks the 
uniqueness of representation of -1 as constm1_rtx.  Can't we find a really 
contained hack instead, especially if we want to backport it to 4.8?

On Thu, Oct 24, 2013 at 10:43 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> The most isolated patch I can come up with, especially since we ought to
>> fix this in 4.8 branch as well, is to only allow expansion of wide int
>> modes to const_int when they're positive.
>>
>> Thoughts?
>
> That's a fairly dangerous hack in my opinion, in particular this breaks the
> uniqueness of representation of -1 as constm1_rtx.  Can't we find a really
> contained hack instead, especially if we want to backport it to 4.8?

Pass the mode through all 10 stackframes is the only good fix given
recent activity on modes vs. const_int.

Richard.

> --
> Eric Botcazou

> That's a fairly dangerous hack in my opinion, in particular this breaks the
> uniqueness of representation of -1 as constm1_rtx.  Can't we find a really
> contained hack instead, especially if we want to backport it to 4.8?

In particular, can't Uros' patch be considered as such here?  Frankly, the 
choice of 'true' vs 'false' for create_convert_operand_to in optabs isn't 
crystal clear...

Eric Botcazou <ebotcazou@adacore.com> writes:
>> That's a fairly dangerous hack in my opinion, in particular this breaks the
>> uniqueness of representation of -1 as constm1_rtx.  Can't we find a really
>> contained hack instead, especially if we want to backport it to 4.8?
>
> In particular, can't Uros' patch be considered as such here?  Frankly, the 
> choice of 'true' vs 'false' for create_convert_operand_to in optabs isn't 
> crystal clear...

Do we actually need to do a conversion here at all?  It looks like the
modes of "expected" and "desired" should already match "mem", so we could
just use create_input_operand.

Thanks,
Richard