Patchwork wide-int branch now up for public comment and review

login
register
mail settings
Submitter Kenneth Zadeck
Date Aug. 24, 2013, 8:46 p.m.
Message ID <52191B8E.3060209@naturalbridge.com>
Download mbox | patch
Permalink /patch/269669/
State New
Headers show

Comments

Kenneth Zadeck - Aug. 24, 2013, 8:46 p.m.
>>> The patch goes for (1) but (2) seems better to me, for a few reasons:
>>>
>>> * As above, constants coming from rtl are already in the right form,
>>>     so if you create a wide_int from an rtx and only query it, no explicit
>>>     extension is needed.
>>>
>>> * Things like logical operations and right shifts naturally preserve
>>>     the sign-extended form, so only a subset of write operations need
>>>     to take special measures.
>> right now the internals of wide-int do not keep the bits above the
>> precision clean.   as you point out this could be fixed by changing
>> lshift, add, sub, mul, div (and anything else i have forgotten about)
>> and removing the code that cleans this up on exit.   I actually do not
>> really care which way we go here but if we do go on keeping the bits
>> clean above the precision inside wide-int, we are going to have to clean
>> the bits in the constructors from rtl, or fix some/a lot of bugs.
>>
>> But if you want to go with the stay clean plan you have to start clean,
>> so at the rtl level this means copying. and it is the not copying trick
>> that pushed me in the direction we went.
>>
>> At the tree level, this is not an issue.   There are no constructors for
>> tree-csts that do not have a type and before we copy the rep from the
>> wide-int to the tree, we clean the top bits.   (BTW - If i had to guess
>> what the bug is with the missing messages on the mips port, it would be
>> because the front ends HAD a bad habit of creating constants that did
>> not fit into a type and then later checking to see if there were any
>> interesting bits above the precision in the int-cst.  This now does not
>> work because we clean out those top bits on construction but it would
>> not surprise me if we missed the fixed point constant path.)   So at the
>> tree level, we could easily go either way here, but there is a cost at
>> the rtl level with doing (2).
> TBH, I think we should do (2) and fix whatever bugs you saw with invalid
> rtx constants.
>
luckily (or perhaps unluckily) if you try the test it fails pretty 
quickly - building gcclib on the x86-64.   I have enclosed a patch to 
check this.    you can try it your self and see if you really think this 
is right path.

good luck, i fear you may need it.

on the other hand, if it is just a few quick bugs, then i will agree 
that (2) is right path.

kenny

Patch

Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc	(revision 201968)
+++ gcc/wide-int.cc	(working copy)
@@ -171,6 +171,10 @@  wide_int_ro::from_rtx (const rtx_mode_t
     case CONST_INT:
       result.val[0] = INTVAL (x);
       result.len = 1;
+
+      if (prec != HOST_BITS_PER_WIDE_INT)
+	gcc_assert (result.val[0] == sext_hwi (result.val[0], prec));
+
 #ifdef DEBUG_WIDE_INT
       debug_wh ("wide_int:: %s = from_rtx ("HOST_WIDE_INT_PRINT_HEX")\n",
 		result, INTVAL (x));