All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: linux-sparse@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: [PATCH] RFC: problems with cast optimization at evaluation time
Date: Sat,  2 Jan 2021 11:09:34 +0100	[thread overview]
Message-ID: <20210102100934.28459-1-luc.vanoostenryck@gmail.com> (raw)

The current code will, at evaluation time, optimize some cast.
For example, code like:
	int i  = <some value>;
	char c = ~i;
will be optimized to what would essentially be:
	int i = <some value>;
	char c = ~(char) i;

This is fine but it has at least two problems:
	int i = <some value>;
	bool b = ~i;
is not the same as the 'optimized' (it's would only be for 0 and -1):
	int i = <some value>;
	bool b = ~(bool)i;
Same with floats:
	int i = <some value>;
	float f = ~i;
in this case the 'optimized' form doesn't even make sense:
	int i = <some value>;
	float f = ~(float)i;

It's easy enough to add a test to only allow this on 'true' integer types
like done in the patch.

However, the same problem also exist for the optimization of the
sequence of two implied cast (only that this is rare and I haven't
succeed to reproduce a case where it is wrong). For example,
a conversion of an integer to a float can overflow, so a conversion
like 'int -> float -> int' is not always a no-op.

Again it's easy enough to add some checks but I wonder if all this
is really worth. My gut feeling is that this should not be done at
evaluation time and should only be done after linearization.
I just wonder if this simplification wasn't done for some specific
purpose other than avoiding an allocation and keeping the AST small?

-- Luc
---
 evaluate.c                       |  2 +-
 validation/eval/not-cast-bool.c  | 14 ++++++++++++++
 validation/eval/not-cast-float.c | 14 ++++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 validation/eval/not-cast-bool.c
 create mode 100644 validation/eval/not-cast-float.c

diff --git a/evaluate.c b/evaluate.c
index 41871e18503a..843b56a0386a 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -337,7 +337,7 @@ static struct expression * cast_to(struct expression *old, struct symbol *type)
 	case EXPR_PREOP:
 		if (old->ctype->bit_size < type->bit_size)
 			break;
-		if (old->op == '~') {
+		if (old->op == '~' && is_int_type(type) && !is_bool_type(type)) {
 			old->ctype = type;
 			old->unop = cast_to(old->unop, type);
 			return old;
diff --git a/validation/eval/not-cast-bool.c b/validation/eval/not-cast-bool.c
new file mode 100644
index 000000000000..acd8bbf293db
--- /dev/null
+++ b/validation/eval/not-cast-bool.c
@@ -0,0 +1,14 @@
+static _Bool foo(void)
+{
+	unsigned char c = 1;
+	_Bool b = ~c;
+	return b;
+}
+
+/*
+ * check-name: not-cast-bool
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-ignore
+ * check-output-returns: 1
+ */
diff --git a/validation/eval/not-cast-float.c b/validation/eval/not-cast-float.c
new file mode 100644
index 000000000000..d474d69bdda3
--- /dev/null
+++ b/validation/eval/not-cast-float.c
@@ -0,0 +1,14 @@
+static int foo(void)
+{
+	int i = 123;
+	float x = ~i;
+	return (x < 0);
+}
+
+/*
+ * check-name: eval-bool-zext-neg
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-ignore
+ * check-output-returns: 1
+ */
-- 
2.29.2


             reply	other threads:[~2021-01-02 10:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-02 10:09 Luc Van Oostenryck [this message]
2021-01-02 19:52 ` [PATCH] RFC: problems with cast optimization at evaluation time Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210102100934.28459-1-luc.vanoostenryck@gmail.com \
    --to=luc.vanoostenryck@gmail.com \
    --cc=linux-sparse@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.