On 12/01/2012 04:28 AM, Richard Sandiford wrote:
Kenneth Zadeck <zad...@naturalbridge.com> writes:
2) The patch does not work for rtxes at all. Rtxes have to copied.
Trees could be pointer copied.
The problem is that CONST_INTs are not canonized in a way that wide-ints
are or that trees could be.
This comes from the use of the GEN_INT macro that does not take a
mode. Without a mode, you do not get the integers into proper form:
i.e. sign extended. At the tree level, this is not a problem because
the INT_CST constructors take a type. But the best that can be done at
the rtl level is to do the sign extension when we convert inside
from_rtx (which takes a precision or a mode).
rtxes must be sign-extended too, even under current rules. If you have
a case where an rtx constant isn't sign-extended for the mode in which
it's being used, then either the constant wasn't created correctly or
the code that's calling from_rtx has got the wrong mode. Those are bugs
even now.
This is enforced in some places already. E.g. if an insn has a QImode
const_int_operand, (const_int 128) will not match (assuming 8 bits
per unit of course).
I can well imagine this patch hits cases that haven't been caught
yet though.
I agree in an ideal world, those canonization rules would be true. But i
am a big believer in "trust but verify" and since there was no
verification, because there is no mode actually provided, it is not
surprising that we fail.
I am simply staking out the position that fixing this is an unreasonable
precondition for wide-int.
FWIW, I think the thing you mention here...
Fixing this is on Richard Sandiford's and my "it would be nice list" but
is certainly out of the question to do as a prereq for this patch.
...is the idea of attaching the mode to the constant, rather than
having to keep track of it separately. That's definitely still
something I'd like to do, but it wouldn't involve any changes to
the canonicalisation rules.
I still agree that abstracting the storage seems like an unnecessary
complication. In the other thread, Richard B said:
The patches introduce a lot more temporary wide-ints (your words) and
at the same time makes construction of them from tree / rtx very expensive
both stack space and compile-time wise. Look at how we for example
compute TREE_INT_CST + 1 - int_cst_binop internally uses double_ints
for the computation and then instantiates a new tree for holding the result.
Now we'd use wide_ints for this requring totally unnecessary copying.
But surely the expensive part of that operation is instantiating the
new tree, with its associated hash table lookup and potential allocation.
Trying to save one or two copies of integers from heap to stack seems
minor compared to that.
I have made the point that the huge number of wide ints created are
actually very cheap because the larger than necessary space is never
initialized and the space is on the stack and goes away very quickly.
this "enhancement" to wide int really makes no sense. Mike i an have
spent a lot of time going down this rat hole, and it is time to put it
to rest.
Richard
