On Mar 21, 2012, at 6:47 AM, Michael Matz wrote:
> Actually, I wouldn't.

Ok, thanks for explaining.  In light of that, I'd just say, I want to change 
the spec, the details don't change any for me, only the terminology I might 
use.  The problem is the spec is causing aborts on valid code, and hence, 
either, the code must be duplicated and fixed, or the code has to be fixed.  I 
don't see any value in duplicating the code, so, I am left with fixing the spec 
so that valid programs produce valid code.

> If the high bit of i1 is set then currently you will get 
> a negative number produced no matter the absolute value of it.

Ok, in the new patch, I'm pushing to change the spec so that the value is sign 
extended and fixing all the code that doesn't conform to that spec.  Richard 
seems to be agreeable with the basic idea, though, we are now sorting out all 
the little details to make that happen.  If there is any down-side or details 
we missed or got wrong, please chime in.

> For large negative numbers you'll get a CONST_DOUBLE, 
> that when interpreted in the large requested mode (which is the only thing 
> that makes sense) is positive.

In the new patch, we use sign extension, and when the high bit is set, the 
value is interpreted as a negative number is a larger mode.  I'll test signed 
and unsigned constants coming in from above to ensure the right thing happens.  
Above the signededness is tracked via the type.  In the rtl constant, it isn't, 
so that code will need an assert to prevent large unsigned values from taking 
this code path.

> Hence all values where i1 is between (HWI)1 << (hwibits-1)) and 
> ((HWI)~0)-1 are the values you're searching for, that show the problem.

Presently, I am fixing _all_ problems shown with those values.  If you know of 
any that we don't address, love to hear about them.

> This positive/negative inconsistency doesn't make sense to allow, and the 
> assert ensures that it isn't allowed.

Don't need the assert when there is no inconsistency, I believe that resolving 
any inconsistencies should remove the need for the assert.

> This would have the seemingly strange effect of disallowing too large 
> modes only for large values, but that's simply a side-effect of 
> CONST_DOUBLE and the whole associated machinery not being able to 
> consistently deal with constants wider than 2*HWI_BITS.

I'll move that assert up to the code that has the type information for the 
constant.

>> if I is 42, we abort.  To back the position that spec must not be 
>> changed, you need to explain at least one thing for which the wrong 
>> thing will happen if the spec did change.  If you want to go down that 
>> path, you will need to furnish one example where badness happens with 0, 
>> not 2, not 3, but 0.
> 
> Huh.  Removing the assert wouldn't only allow 0, but also other values.  
> I don't understand your argumentation: "because for 0 nothing bad happens, 
> that proves that nothing bad happens for any other values which we would 
> also allow, hence I can remove the assert"?

Right, it merely proves that the assert is wrong and needs fixing.  Once you 
accept that, then we progress on exactly what it should be.  This is now all 
mostly moot, given that I'm fine with changing the spec as Richard suggested to 
be a sign-extended constant.  Once we have that nice are concrete definition, 
the any code conflicts with that, is buggy, and we just fix.  Seems like a nice 
way forward to me.

> It of course doesn't prove anything at all.

:-)  Only the point I wanted to make; that 0 is safe.  As such, it proves that 
the spec, as you might call it, is wrong.  Once that spec is proven wrong, 
trivially, fixing it, isn't unreasonable.

Reply via email to