[PATCH] volatile loads are side-effects too

[PATCH] volatile loads are side-effects too

[Luc Van Oostenryck]

Some branch simplifications are only valid if the branch is free
of side-effects. The check is done in flow.c:bb_has_side_effects()
but currently deosn't take in account the fact that a volatile load
is also a side-effect.

Fix this by adding the appropriate check.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 flow.c                                  |  7 +++++++
 validation/optim/volatile-side-effect.c | 13 +++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 validation/optim/volatile-side-effect.c

diff --git a/flow.c b/flow.c
index 6b2c879a8..1144bff4e 100644
--- a/flow.c
+++ b/flow.c
@@ -169,6 +169,13 @@ static int bb_has_side_effects(struct basic_block *bb)
 			/* FIXME! This should take "const" etc into account */
 			return 1;
 
+		case OP_LOAD:
+			if (!insn->type)
+				return 1;
+			if (insn->type->ctype.modifiers & MOD_VOLATILE)
+				return 1;
+			continue;
+
 		case OP_STORE:
 		case OP_CONTEXT:
 			return 1;
diff --git a/validation/optim/volatile-side-effect.c b/validation/optim/volatile-side-effect.c
new file mode 100644
index 000000000..842b72282
--- /dev/null
+++ b/validation/optim/volatile-side-effect.c
@@ -0,0 +1,13 @@
+void foo(int p, volatile int *ptr)
+{
+	p ? : *ptr;
+	p ? : *ptr;
+}
+
+/*
+ * check-name: volatile-side-effect
+ * check-command: test-linearize -Wno-decl $file
+ * check-output-ignore
+ *
+ * check-output-pattern(2): load
+ */
-- 
2.14.0

Re: [PATCH] volatile loads are side-effects too

[Christopher Li]

On Tue, Nov 7, 2017 at 11:59 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Some branch simplifications are only valid if the branch is free
> of side-effects. The check is done in flow.c:bb_has_side_effects()
> but currently deosn't take in account the fact that a volatile load
> is also a side-effect.
>
> Fix this by adding the appropriate check.
>
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Looks good to me.

Chris

Re: [PATCH] volatile loads are side-effects too

[Luc Van Oostenryck]

On Wed, Nov 08, 2017 at 02:55:05AM +0800, Christopher Li wrote:
> On Tue, Nov 7, 2017 at 11:59 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Some branch simplifications are only valid if the branch is free
> > of side-effects. The check is done in flow.c:bb_has_side_effects()
> > but currently deosn't take in account the fact that a volatile load
> > is also a side-effect.
> >
> > Fix this by adding the appropriate check.
> >
> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> 
> Looks good to me.

Thank you for the review.
Thank you for the very fast review.

-- Luc