On Sat, Jul 21, 2018 at 01:08:00PM -0700, Linus Torvalds wrote:
> On Sat, Jul 21, 2018 at 7:33 AM Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> >
> > The goal of this series is:
> > * to give a correct & sensible warning on negative or over-sized shifts.
> > * to add conservative simplification of such shifts.
>
> Looks fine to me, but ...
I'm annoyed with the (current) impossibility, after linearization,
to reliably know if the shift count is negative or not.
But well ... one thing at a time.
> ... it did remind me that we do absolutely horribly
> badly with one common "big shift" case.
>
> Try linearizing this (obviously with -m64):
>
> int test(unsigned long x)
> {
> return x >> 32 >> 32;
> }
>
> int test2(unsigned long x)
> {
> return x >> 64;
> }
>
> and note how the second case warns, but gives the right result. The
> first case is "correct", but gets garbage code generation ;(
>
> So I wish sparse had the proper simplification of "x >> C1 >> C2" into
> "x >> (C1+C2)", but then the end result needs to work properly with
> the big-shift case.
Yes, it was already on my list since a long while.
The following patch should do the job for constant shift counts but:
* it also needs to be done at expand time
* it needs to take care of negative shift counts and overflows
* it needs to have the non-constant cases too.
-- Luc
>From 0b79b79fb853e6eb40b42459a0c479ed9cce11d5 Mon Sep 17 00:00:00 2001
From: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
Date: Sun, 22 Jul 2018 01:23:24 +0200
Subject: [DRAFT] shift: simplify LSR(LSR(x,N),N) & friends
This patch is also available for testing at
git://github.com/lucvoo/sparse-dev.git optim-shift-v0
---
simplify.c | 28 ++++++++-
validation/optim/shift-shift.c | 106 +++++++++++++++++++++++++++++++++
2 files changed, 133 insertions(+), 1 deletion(-)
create mode 100644 validation/optim/shift-shift.c
diff --git a/simplify.c b/simplify.c
index 3a5ae9331..36ac0f427 100644
--- a/simplify.c
+++ b/simplify.c
@@ -543,6 +543,8 @@ undef:
static int simplify_shift(struct instruction *insn, pseudo_t pseudo, long long value)
{
unsigned int size;
+ pseudo_t src2;
+ int rc = 0;
if (!value)
return replace_with_pseudo(insn, pseudo);
@@ -555,15 +557,39 @@ static int simplify_shift(struct instruction *insn, pseudo_t pseudo, long long v
}
switch (insn->opcode) {
case OP_ASR:
+ switch(def_opcode(pseudo)) {
+ case OP_ASR:
+ src2 = pseudo->def->src2;
+ if (src2->type == PSEUDO_VAL) {
+ insn->tainted = 1;
+ value += src2->value;
+ if (value >= insn->size)
+ value = insn->size - 1;
+ insn->src2 = value_pseudo(value);
+ replace_pseudo(insn, &insn->src1, pseudo->def->src1);
+ return REPEAT_CSE;
+ }
+ break;
+ }
break;
case OP_LSR:
size = operand_size(insn, pseudo);
/* fall through */
case OP_SHL:
+ if (def_opcode(pseudo) == insn->opcode) {
+ insn->tainted = 1;
+ src2 = pseudo->def->src2;
+ if (src2->type == PSEUDO_VAL) {
+ value += src2->value;
+ insn->src2 = value_pseudo(value);
+ replace_pseudo(insn, &insn->src1, pseudo->def->src1);
+ rc |= REPEAT_CSE;
+ }
+ }
if (value >= size)
return replace_with_pseudo(insn, value_pseudo(0));
}
- return 0;
+ return rc;
}
static int simplify_mul_div(struct instruction *insn, long long value)
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
