| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
In simplify_loads(), phisrcs are created in find_dominating_parents()
and are then supposed to be used in rewrite_load_instruction().
However, it may happen (quite often) that find_dominating_parents()
find a dominator for one of the branch, create a phi-source for it,
record it's usage and then doesn't find a dominator in one of other
parent branches. In this case, the function returns early and the
created phisrcs are simply ignored. These phisrcs can't be simplified
away as dead instructions because they still have their usage recorded.
Fix this by explicitly remove these ignored phisrcs.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The function address_taken() check that the address of a variable
is not used for anything else than for doing a memory access.
If the function fails, it means that the address can escape and
thus can be used to modify the variable indirectly and the
memop simplification would then be unsafe.
The function do this check by assuming that any uses by an instruction
other than a load or a store can escape the address, which is true.
However it doesn't take in account that if the variable's address is
used, not as the address of a store, but as the value stored, then
this address also escape.
Fix this by adding a check that the use of the variable's address
is effectively done as the address of stores & loads and not as
the value stored by the memop.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In the initial simplify_symbols() and in simplify_memops()
when a store is simplified away, it's killed via kill_store()
where its ->bb is set to NULL and the usage is removed from
the value. However the usage is not removed from the address.
As consequence, code related to the address calculation
is not optimized away as it should be since the value is
wrongly considered as needed.
Fix this by using kill_instruction_force() to remove these
stores.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently, OP_PHISOURCEs are given a size but not a type.
For consistency and for sparse-LLVM which need it,
give them a type too.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
kill_dominated_stores() simplify away redundant stores.
Nice but volatile stores are never redundant and must
never be simplified away.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In memops:find_dominating_parents(), the 'load-load optimization'
(see commit cf07903a "Don't bother finding dominating loads if ...")
cause some loads simplification to be missed.
For example, with the following code:
int foo(int *i, int *j)
{
*i = 6;
*j = 1;
do {
if (*i != 6)
(*i)++;
(*i)++;
} while (*i != *j);
return *j;
}
test-linearize returns something like:
foo:
.L0:
<entry-point>
store.32 $6 -> 0[%arg1]
store.32 $1 -> 0[%arg2]
br .L1
.L1:
load.32 %r4 <- 0[%arg1]
setne.32 %r5 <- %r4, $6
br %r5, .L4, .L5
.L4:
add.32 %r8 <- %r4, $1
store.32 %r8 -> 0[%arg1]
br .L5
.L5:
load.32 %r10 <- 0[%arg1]
add.32 %r11 <- %r10, $1
store.32 %r11 -> 0[%arg1]
load.32 %r15 <- 0[%arg2]
setne.32 %r16 <- %r11, %r15
br %r16, .L1, .L6
.L6:
ret.32 %r15
where we can notice that the first load in .L5 is not needed,
the value could be retrieved from %r4 and %r8, like:
@@ -8,15 +8,17 @@
.L1:
load.32 %r4 <- 0[%arg1]
setne.32 %r5 <- %r4, $6
+ phisrc.32 %phi4 <- %r4
br %r5, .L4, .L5
.L4:
add.32 %r8 <- %r4, $1
store.32 %r8 -> 0[%arg1]
+ phisrc.32 %phi5 <- %r8
br .L5
.L5:
- load.32 %r10 <- 0[%arg1]
+ phi.32 %r10 <- %phi4, %phi5
add.32 %r11 <- %r10, $1
store.32 %r11 -> 0[%arg1]
load.32 %r15 <- 0[%arg2]
The fix essentially consists in reverting commit cf07903a but on
memops.c's version of find_dominating_parents().
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
memops.c:simplify_loads() tries to simplify all loads,
even volatile ones.
For example, on the following code:
static int foo(volatile int *a, int v)
{
*a = v;
return *a;
}
test-linearize returns something like:
foo:
store.32 %arg2 -> 0[%arg1]
ret.32 %arg2
while the correct output is more like:
foo:
store.32 %arg2 -> 0[%arg1]
load.32 %r5 <- 0[%arg1]
ret.32 %r5
The fix is to simply ignore loads with the 'volatile' modifier.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
I noticed that the second part of this conditional is always true.
Just a shot in the dark: Could that be a typo?
Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Josh Triplett <josh@freedesktop.org>
|
|
The current way of tracking pseudo register user is by
keeping a list of the address of the pseudo_t member.
This address can be in part of the instruction member, the
worse case is in the argument list of the call instruction.
As the comment for address_taken() said, using the container
to get instruction pointer is wrong. It use to work with instruction
that relate to symbol address. But that is not true any more. Even
worse, it is very hard to track the pseudo usage other than symbol
address. The only reason symbol address used to works for call instruction
is because call instruction did not directly use the symbol address.
I bit the bullet and just add the instruction pointer to pair with
the pseudo user pointer. So it will work with the case that the
user instruction is call as well.
Testing:
I compare the linearize result with/without the patch on a few
sparse source file it self. The linearize generate exactly
the same result except the symbol address changes. Which is
predictable different because the pseudo user structure allocate
memory.
Singed-Off-By: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Pavel Roskin <proski@gnu.org>
Signed-off-by: Josh Triplett <josh@freedesktop.org>
|
|
This shows that we have some inlining bug where we seem to
be corrupting the offsetting of an array. So right now the
warnings we get in the kernel seem to be bogus, but this
should help find that other bug too, and it doesn't trigger
often enough to be too distracting.
|
|
with the OP_SYMADDR simplifications.
It's still wrong. Now it just happens to work in most cases.
|
|
(symbol addresses).
They are pretty different. Symbol addresses have special meaning during
various phases, from symbol simplification to CSE.
|
|
the entry bb magical, just make the OP_ENTRY instruction act
like an OP_CALL from a dominance standpoint: it dominates all
non-local memory operations.
This removes yet another special case.
|
|
entry basic block in memop dominators.
This caused the dominator finding to totally misjudge a lot of
non-local loads as having no dominators (or thinking they were
being completely dominated by a store that just updated them).
|
|
This is purely for debugging. It's only used to show what symbol
a pseudo might be (and I stress "might", since CSE can and does
screw it up) associated with.
Also print out the def list for a basic block when verbose.
It all makes it a bit easier to guess what's up.
|
|
We should drop symbol types by now, and base things on just raw
sizes. Anything else just doesn't work from a CSE standpoint.
Some things may care about actual types later, notably the type-
based alias analysis. Those types still exist in the cast nodes.
Also, make the printout be more readable.
|
|
to it, it's not correct to assume that a bb without parents
is the entrypoint.
|
|
It's a bit more simple-minded than the symbol simplification,
and it can be more costly. So we start off with the (cheap)
symbol simplification algorithm, and then use this new more
generic phase later.
This allows us to remove extra loads (and, in theory, stores,
but the dead store elimination is so simple-minded right now
that it's effectively useless).
|
