Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi Richard, Do you think this version is good enough to commit to trunk? I have asked Andrew to commit for me. If it is not good enough, I may ask him not to do this. --- Lin Zuojian
Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi, Currently warning about strict aliasing is not strict enough. It has not considered tbaa. So a test case emit addition error. This patch is not good for commit. -- Lin Zuojian
Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi, My patch for warning about strict aliasing violation is not strict enough. Current implementation of strict_aliasing_warn has not consider tbaa. So a test case gcc/testsuite/g++.dg/opt/pmf1.C will emit additional warning whereas it shouldn't do. My patch is not good for committing. --- Lin Zuojian
Re: [PATCH v3] Add strict aliasing warning when inlining function.
Hi,
This version make sure that the added test case pass the test.
* tree-inline.c (setup_one_parameter): Add strict aliasing check.
* c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
* c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
* alias.c (strict_aliasing_warning): New function moved from
c-family/c-common.c.
* tree.h (strict_aliasing_warning): New function declaration moved from
c-family/c-common.h.
* testsuite/g++.dg/warn/Wstrict-aliasing-inline-parameter.C: New test.
---
gcc/ChangeLog | 11 +++
gcc/alias.c| 78 ++
gcc/c-family/c-common.c| 78 --
gcc/c-family/c-common.h| 1 -
.../warn/Wstrict-aliasing-inline-parameter.C | 40 +++
gcc/tree-inline.c | 4 ++
gcc/tree.h | 2 +
7 files changed, 135 insertions(+), 79 deletions(-)
create mode 100644
gcc/testsuite/g++.dg/warn/Wstrict-aliasing-inline-parameter.C
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e1b655f..4338e2b 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2014-08-21 Lin Zuojian manjian2...@gmail.com
+
+ * tree-inline.c (setup_one_parameter): Add strict aliasing check.
+ * c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
+ * c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
+ * alias.c (strict_aliasing_warning): New function moved from
+ c-family/c-common.c.
+ * tree.h (strict_aliasing_warning): New function declaration moved from
+ c-family/c-common.h.
+ * testsuite/g++.dg/warn/Wstrict-aliasing-inline-parameter.C: New test.
+
2014-08-20 David Malcolm dmalc...@redhat.com
* cfgrtl.c (duplicate_insn_chain): Convert the checked cast on
diff --git a/gcc/alias.c b/gcc/alias.c
index 39df09b..8496435 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -3044,4 +3044,82 @@ end_alias_analysis (void)
sbitmap_free (reg_known_equiv_p);
}
+/* Print a warning about casts that might indicate violation
+ of strict aliasing rules if -Wstrict-aliasing is used and
+ strict aliasing mode is in effect. OTYPE is the original
+ TREE_TYPE of EXPR, and TYPE the type we're casting to. */
+
+bool
+strict_aliasing_warning (tree otype, tree type, tree expr)
+{
+ /* Strip pointer conversion chains and get to the correct original type. */
+ STRIP_NOPS (expr);
+ otype = TREE_TYPE (expr);
+
+ if (!(flag_strict_aliasing
+POINTER_TYPE_P (type)
+POINTER_TYPE_P (otype)
+!VOID_TYPE_P (TREE_TYPE (type)))
+ /* If the type we are casting to is a ref-all pointer
+ dereferencing it is always valid. */
+ || TYPE_REF_CAN_ALIAS_ALL (type))
+return false;
+
+ if ((warn_strict_aliasing 1) TREE_CODE (expr) == ADDR_EXPR
+ (DECL_P (TREE_OPERAND (expr, 0))
+ || handled_component_p (TREE_OPERAND (expr, 0
+{
+ /* Casting the address of an object to non void pointer. Warn
+ if the cast breaks type based aliasing. */
+ if (!COMPLETE_TYPE_P (TREE_TYPE (type)) warn_strict_aliasing == 2)
+ {
+ warning (OPT_Wstrict_aliasing, type-punning to incomplete type
+ might break strict-aliasing rules);
+ return true;
+ }
+ else
+{
+ /* warn_strict_aliasing = 3. This includes the default (3).
+ Only warn if the cast is dereferenced immediately. */
+ alias_set_type set1 =
+ get_alias_set (TREE_TYPE (TREE_OPERAND (expr, 0)));
+ alias_set_type set2 = get_alias_set (TREE_TYPE (type));
+
+ if (set1 != set2 set2 != 0
+ (set1 == 0 || !alias_sets_conflict_p (set1, set2)))
+ {
+ warning (OPT_Wstrict_aliasing, dereferencing type-punned
+ pointer will break strict-aliasing rules);
+ return true;
+ }
+ else if (warn_strict_aliasing == 2
+ !alias_sets_must_conflict_p (set1, set2))
+ {
+ warning (OPT_Wstrict_aliasing, dereferencing type-punned
+ pointer might break strict-aliasing rules);
+ return true;
+ }
+}
+}
+ else
+if ((warn_strict_aliasing == 1) !VOID_TYPE_P (TREE_TYPE (otype)))
+ {
+/* At this level, warn for any conversions, even if an address is
+ not taken in the same statement. This will likely produce many
+ false positives, but could be useful to pinpoint problems that
+ are not revealed at higher levels. */
+alias_set_type set1 = get_alias_set (TREE_TYPE (otype));
+alias_set_type set2 = get_alias_set (TREE_TYPE (type));
+if (!COMPLETE_TYPE_P (type)
+|| !alias_sets_must_conflict_p
Re: [PATCH v3] Add strict aliasing warning when inlining function.
Hi, And I have run test like this: make check-g++ RUNTESTFLAGS=dg.exp And turned out there are 3 more unexpected failures like g++.dg/opt/pmf1.C /home/linzj/src/build-gcc/gcc-trunk/gcc/testsuite/g++.dg/opt/pmf1.C: In function ‘int main()’: /home/linzj/src/build-gcc/gcc-trunk/gcc/testsuite/g++.dg/opt/pmf1.C:72:42: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] t.forward(itemfunptr, Item::fred, 1); ^ /home/linzj/src/build-gcc/gcc-trunk/gcc/testsuite/g++.dg/opt/pmf1.C:72:42: warning: during inlining function void WorldObjectDerived::forward(memfunT, arg1T, arg2T) [with memfunT = void (Container::*)(void (Item::*)(int), int); arg1T = void (Item::*)(int); arg2T = int; Derived = Container] into function int main() [-Wstrict-aliasing] FAIL: g++.dg/opt/pmf1.C -std=gnu++98 (test for excess errors) PASS: g++.dg/opt/pmf1.C -std=gnu++98 execution test FAIL: g++.dg/opt/pmf1.C -std=gnu++11 (test for excess errors) PASS: g++.dg/opt/pmf1.C -std=gnu++11 execution test FAIL: g++.dg/opt/pmf1.C -std=gnu++1y (test for excess errors) I think it's okay to fix this test. And commit my code. --- Lin Zuojian
[PATCH v4] Add strict aliasing warning when inlining function.
Hi,
This patch is to improve the output of strict_aliasing_warning. This
version will output the expression of rhs, type of that expression,
and the type of lhs. That make it easier to debug strict aliasing
bug.
* tree-inline.c (setup_one_parameter): Add strict aliasing check.
* c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
* c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
* alias.c (strict_aliasing_warning): New function moved from
c-family/c-common.c.
* tree.h (strict_aliasing_warning): New function declaration moved from
c-family/c-common.h.
* testsuite/g++.dg/warn/Wstrict-aliasing-inline-parameter.C: New test.
---
gcc/ChangeLog | 11 +++
gcc/alias.c| 107 +
gcc/c-family/c-common.c| 78 ---
gcc/c-family/c-common.h| 1 -
.../warn/Wstrict-aliasing-inline-parameter.C | 40
gcc/tree-inline.c | 4 +
gcc/tree.h | 2 +
7 files changed, 164 insertions(+), 79 deletions(-)
create mode 100644
gcc/testsuite/g++.dg/warn/Wstrict-aliasing-inline-parameter.C
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1067203..bff6354 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2014-08-21 Lin Zuojian manjian2...@gmail.com
+
+ * tree-inline.c (setup_one_parameter): Add strict aliasing check.
+ * c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
+ * c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
+ * alias.c (strict_aliasing_warning): New function moved from
+ c-family/c-common.c.
+ * tree.h (strict_aliasing_warning): New function declaration moved from
+ c-family/c-common.h.
+ * testsuite/g++.dg/warn/Wstrict-aliasing-inline-parameter.C: New test.
+
2014-08-21 Manuel López-Ibáñez m...@gcc.gnu.org
PR fortran/44054
diff --git a/gcc/alias.c b/gcc/alias.c
index 39df09b..ab11ae3 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3. If not see
#include is-a.h
#include gimple.h
#include gimple-ssa.h
+#include tree-pretty-print.h
/* The aliasing API provided here solves related but different problems:
@@ -3044,4 +3045,110 @@ end_alias_analysis (void)
sbitmap_free (reg_known_equiv_p);
}
+/* Helper of strict_aliasing_warning to format warning. */
+static void
+strict_aliasing_warning_output (tree type, tree expr, const char *msg)
+{
+ pretty_printer pp;
+ pp_string (pp, msg);
+ pp_dot (pp);
+ pp_space (pp);
+ pp_string (pp, With expression);
+ pp_colon (pp);
+ pp_space (pp);
+ dump_generic_node (pp, expr, 0, 0, false);
+ pp_comma (pp);
+ pp_space (pp);
+ pp_string (pp, type of expresssion);
+ pp_colon (pp);
+ pp_space (pp);
+ dump_generic_node (pp, TREE_TYPE (expr), 0, 0, false);
+ pp_comma (pp);
+ pp_space (pp);
+ pp_string (pp, type to cast);
+ pp_colon (pp);
+ pp_space (pp);
+ dump_generic_node (pp, type, 0, 0, false);
+ pp_dot (pp);
+ warning (OPT_Wstrict_aliasing, pp_formatted_text (pp), NULL);
+}
+
+/* Print a warning about casts that might indicate violation
+ of strict aliasing rules if -Wstrict-aliasing is used and
+ strict aliasing mode is in effect. OTYPE is the original
+ TREE_TYPE of EXPR, and TYPE the type we're casting to. */
+
+bool
+strict_aliasing_warning (tree otype, tree type, tree expr)
+{
+ /* Strip pointer conversion chains and get to the correct original type. */
+ STRIP_NOPS (expr);
+ otype = TREE_TYPE (expr);
+
+ if (!(flag_strict_aliasing
+POINTER_TYPE_P (type)
+POINTER_TYPE_P (otype)
+!VOID_TYPE_P (TREE_TYPE (type)))
+ /* If the type we are casting to is a ref-all pointer
+ dereferencing it is always valid. */
+ || TYPE_REF_CAN_ALIAS_ALL (type))
+return false;
+
+ if ((warn_strict_aliasing 1) TREE_CODE (expr) == ADDR_EXPR
+ (DECL_P (TREE_OPERAND (expr, 0))
+ || handled_component_p (TREE_OPERAND (expr, 0
+{
+ /* Casting the address of an object to non void pointer. Warn
+ if the cast breaks type based aliasing. */
+ if (!COMPLETE_TYPE_P (TREE_TYPE (type)) warn_strict_aliasing == 2)
+ {
+ strict_aliasing_warning_output (type, expr, type-punning to
incomplete type
+ might break strict-aliasing rules);
+ return true;
+ }
+ else
+{
+ /* warn_strict_aliasing = 3. This includes the default (3).
+ Only warn if the cast is dereferenced immediately. */
+ alias_set_type set1 =
+ get_alias_set (TREE_TYPE (TREE_OPERAND (expr, 0)));
+ alias_set_type set2 = get_alias_set (TREE_TYPE (type));
+
+ if (set1 != set2 set2 != 0
+ (set1 == 0 || !alias_sets_conflict_p
Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi Andrew, What do you think of the current patch? If you thought it was find to commit to trunk, please commit it for me. -- Lin Zuojian
Re: [PATCH v2] Add strict aliasing warning when inlining function.
Hi Richard, Generally I don't think we want to expand the use of the IMHO broken strict_aliasing_warning code. If you have read my test code, you must understand it is the programmer who responsible for this undefined behavior, instead of the compiler. Like PR 60546 concluded. And I doubt if limiting the compiler behavior is a good choice. We should be able to do better after some constant/forward propagation, scanning for MEM_REFs operating on decls and accessing those with an alias set that is not a subset of the set of the decl. Like simply warn as part of the tree-ssa-forwprop.c sweep (maybe only for those MEM_REFs we simplified to avoid duplicates, but then also watch for CCP doing the same). That's a more generalize solution. But it have a defection: at that point, the compiler have no idea who generates these code. That make it difficult to debug. --- Lin Zuojian
Re: [PATCH v2] Add strict aliasing warning when inlining function.
Here is the warning after my patch: 1.cpp: In function 'int foo(int, int, int)': 1.cpp:29:70: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] return hash2(static_castconst unsigned short*(change1), len / 2); ^ 1.cpp:29:70: warning: during inlining function int hash2(const short unsigned int*, int) into function int foo(int, int, int) [-Wstrict-aliasing] It is clear what is going on. On Tue, Aug 19, 2014 at 04:49:57PM +0800, lin zuojian wrote: Hi Richard, Generally I don't think we want to expand the use of the IMHO broken strict_aliasing_warning code. If you have read my test code, you must understand it is the programmer who responsible for this undefined behavior, instead of the compiler. Like PR 60546 concluded. And I doubt if limiting the compiler behavior is a good choice. We should be able to do better after some constant/forward propagation, scanning for MEM_REFs operating on decls and accessing those with an alias set that is not a subset of the set of the decl. Like simply warn as part of the tree-ssa-forwprop.c sweep (maybe only for those MEM_REFs we simplified to avoid duplicates, but then also watch for CCP doing the same). That's a more generalize solution. But it have a defection: at that point, the compiler have no idea who generates these code. That make it difficult to debug. --- Lin Zuojian
[PATCH] Add strict aliasing warning when inlining function.
Hi,
I want to add a warning to inlining function when violate strict
aliasing.
See bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60546
For details.
* tree-inline.c (global): Add new include to c-family/c-common.h. To
access strict aliasing check.
(setup_one_parameter): Add strict aliasing check.
* testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C:New test.
---
gcc/ChangeLog | 5 +++
.../gcc.dg/Wstrict-aliasing-inline-parameter.C | 42 ++
gcc/tree-inline.c | 6
3 files changed, 53 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 8ccdde2..f584567 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2014-08-19 Lin Zuojian manjian2...@gmail.com
+ * tree-inline.c (global): Add new include to c-family/c-common.h. To
+ access strict aliasing check.
+ (setup_one_parameter): Add strict aliasing check.
+ * testsuite/gcc.dg/Wstrict-aliasing-inline.C: New test.
2014-08-19 David Malcolm dmalc...@redhat.com
* basic-block.h (BB_HEAD): Convert to a function. Strengthen the
diff --git a/gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
b/gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
new file mode 100644
index 000..9a667be
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -Wstrict-aliasing=10 -fstrict-aliasing } */
+
+struct A
+{
+int a;
+int b;
+int c;
+};
+
+static inline int hash2(const unsigned short* change2, int len)
+{
+int result = 0;
+for(int i = 0; i len; ++i) {
+
+result += change2[i];
+result ^= result 11;
+result += result 17;
+
+// Force avalanching of final 31 bits.
+result ^= result 3;
+result += result 5;
+result ^= result 2;
+result += result 15;
+result ^= result 10;
+}
+return result;
+}
+
+static inline int hash(const void* change1, int len)
+{
+return hash2(static_castconst unsigned short*(change1), len / 2);
+}
+
+
+int foo(int a, int b, int c)
+{
+struct A a_struct = {a, b, c};
+return hash(a_struct, sizeof(struct A));
+}
+
+/* { dg-message dereferencing type-punned pointer will break strict-aliasing
rules } */
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index b6ecaa4..33d7017 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -22,6 +22,8 @@ along with GCC; see the file COPYING3. If not see
#include system.h
#include coretypes.h
#include tm.h
+/* For warn_strict_aliasing. */
+#include c-family/c-common.h
#include diagnostic-core.h
#include tree.h
#include stor-layout.h
@@ -2913,6 +2915,10 @@ setup_one_parameter (copy_body_data *id, tree p, tree
value, tree fn,
}
}
+ if (warn_strict_aliasing 2)
+if (strict_aliasing_warning (TREE_TYPE (rhs), TREE_TYPE(p), rhs))
+ warning (OPT_Wstrict_aliasing, during inlining function %s into
function %s, fndecl_name(fn), function_name(cfun));
+
/* Make an equivalent VAR_DECL. Note that we must NOT remap the type
here since the type of this decl must be visible to the calling
function. */
--
1.9.1
Lin Zuojian
Re: [PATCH] Add strict aliasing warning when inlining function.
Hi Andrew, I configure my gcc only to c language family. Sorry for my sloppy. I have not run the test yet either. I only run my test manually. That will be patch v2 soon. -- Lin Zuojian
Re: [PATCH v2] Add strict aliasing warning when inlining function.
Hi,
Here patch v2. Move the function as Andrew instructed.
* tree-inline.c (setup_one_parameter): Add strict aliasing check.
* c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
* c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
* alias.c (strict_aliasing_warning): New function moved from
c-family/c-common.c.
* tree.h (strict_aliasing_warning): New function declaration moved from
c-family/c-common.h.
* testsuite/gcc.dg/Wstrict-aliasing-inline.C: New test.
---
gcc/ChangeLog | 11 +++
gcc/alias.c| 78 ++
gcc/c-family/c-common.c| 78 --
gcc/c-family/c-common.h| 1 -
.../gcc.dg/Wstrict-aliasing-inline-parameter.C | 42
gcc/tree-inline.c | 4 ++
gcc/tree.h | 2 +
7 files changed, 137 insertions(+), 79 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wstrict-aliasing-inline-parameter.C
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 8ccdde2..6514313 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2014-08-19 Lin Zuojian manjian2...@gmail.com
+
+ * tree-inline.c (setup_one_parameter): Add strict aliasing check.
+ * c-family/c-common.c (strict_aliasing_warning): Move to alias.c.
+ * c-family/c-common.h (strict_aliasing_warning): Move to tree.h.
+ * alias.c (strict_aliasing_warning): New function moved from
+ c-family/c-common.c.
+ * tree.h (strict_aliasing_warning): New function declaration moved from
+ c-family/c-common.h.
+ * testsuite/gcc.dg/Wstrict-aliasing-inline.C: New test.
+
2014-08-19 David Malcolm dmalc...@redhat.com
* basic-block.h (BB_HEAD): Convert to a function. Strengthen the
diff --git a/gcc/alias.c b/gcc/alias.c
index 39df09b..8496435 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -3044,4 +3044,82 @@ end_alias_analysis (void)
sbitmap_free (reg_known_equiv_p);
}
+/* Print a warning about casts that might indicate violation
+ of strict aliasing rules if -Wstrict-aliasing is used and
+ strict aliasing mode is in effect. OTYPE is the original
+ TREE_TYPE of EXPR, and TYPE the type we're casting to. */
+
+bool
+strict_aliasing_warning (tree otype, tree type, tree expr)
+{
+ /* Strip pointer conversion chains and get to the correct original type. */
+ STRIP_NOPS (expr);
+ otype = TREE_TYPE (expr);
+
+ if (!(flag_strict_aliasing
+POINTER_TYPE_P (type)
+POINTER_TYPE_P (otype)
+!VOID_TYPE_P (TREE_TYPE (type)))
+ /* If the type we are casting to is a ref-all pointer
+ dereferencing it is always valid. */
+ || TYPE_REF_CAN_ALIAS_ALL (type))
+return false;
+
+ if ((warn_strict_aliasing 1) TREE_CODE (expr) == ADDR_EXPR
+ (DECL_P (TREE_OPERAND (expr, 0))
+ || handled_component_p (TREE_OPERAND (expr, 0
+{
+ /* Casting the address of an object to non void pointer. Warn
+ if the cast breaks type based aliasing. */
+ if (!COMPLETE_TYPE_P (TREE_TYPE (type)) warn_strict_aliasing == 2)
+ {
+ warning (OPT_Wstrict_aliasing, type-punning to incomplete type
+ might break strict-aliasing rules);
+ return true;
+ }
+ else
+{
+ /* warn_strict_aliasing = 3. This includes the default (3).
+ Only warn if the cast is dereferenced immediately. */
+ alias_set_type set1 =
+ get_alias_set (TREE_TYPE (TREE_OPERAND (expr, 0)));
+ alias_set_type set2 = get_alias_set (TREE_TYPE (type));
+
+ if (set1 != set2 set2 != 0
+ (set1 == 0 || !alias_sets_conflict_p (set1, set2)))
+ {
+ warning (OPT_Wstrict_aliasing, dereferencing type-punned
+ pointer will break strict-aliasing rules);
+ return true;
+ }
+ else if (warn_strict_aliasing == 2
+ !alias_sets_must_conflict_p (set1, set2))
+ {
+ warning (OPT_Wstrict_aliasing, dereferencing type-punned
+ pointer might break strict-aliasing rules);
+ return true;
+ }
+}
+}
+ else
+if ((warn_strict_aliasing == 1) !VOID_TYPE_P (TREE_TYPE (otype)))
+ {
+/* At this level, warn for any conversions, even if an address is
+ not taken in the same statement. This will likely produce many
+ false positives, but could be useful to pinpoint problems that
+ are not revealed at higher levels. */
+alias_set_type set1 = get_alias_set (TREE_TYPE (otype));
+alias_set_type set2 = get_alias_set (TREE_TYPE (type));
+if (!COMPLETE_TYPE_P (type)
+|| !alias_sets_must_conflict_p (set1, set2))
+ {
+warning
Re: [PATCH] PR rtl-optimization/61712
Thanks Richard, I didn't aware that patch. -- Lin Zuojian
[PATCH] PR rtl-optimization/61712
Hi, This crash is due to fail to consider the exception situation that the insn variable may not be a insn at all. arm.c (thumb1_reorg): if the selected insn is not a insn, continue to next bb. --- gcc/config/arm/arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 89684bb..50ae64b 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -16720,7 +16720,7 @@ thumb1_reorg (void) insn = PREV_INSN (insn); /* Find the last cbranchsi4_insn in basic block BB. */ - if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn) + if (!INSN_P (insn) || (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn)) continue; /* Get the register with which we are comparing. */ -- 1.9.1
Re: [PATCH] Dumping Fields of A Class When -fdump-class-hierarchy
Hi,
I have no idea why nobody show his interest in my patch, but it's
very helpful to boost the efficiency of debugging optimized binary
and help recognizing which expression the crash happens in.
For example:
struct A
{
int a;
};
struct B
{
int b;
};
struct C : public A, public B
{
struct
{
int s;
} s;
int c;
};
would generates the following content:
Class A
size=4 align=4
base size=4 base align=4
A (0x0x7fd7ed5564e0) 0
fields:
int a;
size: 32, bpos: 0
Class B
size=4 align=4
base size=4 base align=4
B (0x0x7fd7ed556540) 0
fields:
int b;
size: 32, bpos: 0
Class C::anonymous struct
size=4 align=4
base size=4 base align=4
C::anonymous struct (0x0x7fd7ed556660) 0
fields:
int s;
size: 32, bpos: 0
Class C
size=16 align=4
base size=16 base align=4
C (0x0x7fd7ef215e70) 0
A (0x0x7fd7ed5565a0) 0
B (0x0x7fd7ed556600) 4
fields:
A (base);
size: 32, bpos: 0
B (base);
size: 32, bpos: 32
C::anonymous struct s;
size: 32, bpos: 64
int c;
size: 32, bpos: 96
That helps to identify the field access, the assembly:
movl12(%rax), %edx
may mean accessing the field c of struct C.
It helps a lot.
--
lin zuojian
[PATCH] Dumping Fields of A Class When -fdump-class-hierarchy
* class.c (dump_class_hierarchy_1): dump fields after hierarchy information.
---
gcc/cp/class.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 14780e7..13579ac 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -8202,6 +8202,44 @@ dump_class_hierarchy_1 (FILE *stream, int flags, tree t)
(unsigned long)(TYPE_ALIGN (CLASSTYPE_AS_BASE (t))
/ BITS_PER_UNIT));
dump_class_hierarchy_r (stream, flags, TYPE_BINFO (t), TYPE_BINFO (t), 0);
+ if (TYPE_P (t))
+{
+ enum tree_code code;
+
+ code = TREE_CODE (t);
+ if (code == RECORD_TYPE || code == UNION_TYPE)
+{
+ tree field;
+ fprintf (stream, fields:\n);
+ for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+{
+ tree field_type;
+ tree field_name;
+ tree size;
+ tree bpos;
+ enum tree_code code;
+
+ code = TREE_CODE (field);
+ if (code != FIELD_DECL)
+continue;
+ field_type = TREE_TYPE (field);
+ field_name = DECL_NAME (field);
+ size = DECL_SIZE (field);
+
+ if (DECL_FIELD_OFFSET (field))
+bpos = bit_position (field);
+ else
+bpos = NULL;
+ fprintf (
+ stream, %s %s;\n
+ size: %ld, bpos: %ld\n,
+ type_as_string (field_type, TFF_PLAIN_IDENTIFIER),
+ field_name ? IDENTIFIER_POINTER (field_name) : (base),
+ tree_to_shwi (size), bpos == NULL ? 0 : tree_to_shwi (bpos));
+}
+}
+}
+
fprintf (stream, \n);
}
--
1.9.1
Re: [PATCH v8] PR middle-end/60281
Hi Jakub, On Fri, Apr 18, 2014 at 09:09:17AM +0200, Jakub Jelinek wrote: Extra line missing before the PR line. Should I post PATCH v9 or someone helps adding one when committing the patch? -- Regards lin zuojian
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, I have my copyright mark signed and the process has completed. Now I am going to answer two more questions before my patch can be commited right? Did you copy any files or text written by someone else in these changes?” no [Which files have you changed so far, and which new files have you written so far?] gcc/asan.c gcc/ChangeLog gcc/cfgexpand.c Okay, you may review my patch again, if there is no problem, please commit it for me. -- Regards lin zuojian
[PATCH v8] PR middle-end/60281
Hi,
Here is the patch after the Jakub's review, and Jakub helps with the
coding style.
--
* asan.c (asan_emit_stack_protection):
Force the base to align to appropriate bits if STRICT_ALIGNMENT. Set
shadow_mem align to appropriate bits if STRICT_ALIGNMENT.
* cfgexpand.c
(expand_stack_vars): Set base_align appropriately when asan is on.
(expand_used_vars): Leave a space in the stack frame for alignment if
STRICT_ALIGNMENT.
---
gcc/ChangeLog | 9 +
gcc/asan.c | 15 +++
gcc/cfgexpand.c | 18 --
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index da35be8..30a2b33 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2014-04-18 Lin Zuojian manjian2...@gmail.com
+ PR middle-end/60281
+ * asan.c (asan_emit_stack_protection): Force the base to align to
+ appropriate bits if STRICT_ALIGNMENT. Set shadow_mem align to
+ appropriate bits if STRICT_ALIGNMENT.
+ * cfgexpand.c (expand_stack_vars): Set base_align appropriately
+ when asan is on.
+ (expand_used_vars): Leave a space in the stack frame for alignment
+ if STRICT_ALIGNMENT.
2014-04-17 Jakub Jelinek ja...@redhat.com
PR target/60847
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..28a476f 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,17 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+base = expand_binop (Pmode, and_optab, base,
+gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
+ ASAN_SHADOW_SHIFT)
+/ BITS_PER_UNIT), Pmode), NULL_RTX,
+1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1106,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned
int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
@@ -1186,6 +1197,10 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+ if (STRICT_ALIGNMENT)
+set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
prev_offset = base_offset;
last_offset = base_offset;
last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index b7f6360..14511e1 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,19 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX (crtl-max_used_stack_slot_alignment,
+ GET_MODE_ALIGNMENT (SImode)
+ ASAN_SHADOW_SHIFT);
}
else
- offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ {
+ offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1845,6 +1854,11 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
+ ASAN_SHADOW_SHIFT)
+/ BITS_PER_UNIT, 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
--
1.8.3.2
--
Regards
lin zuojian
Re: [PATCH v8] PR middle-end/60281
Hi Bernd,
a) On which target(s) did you boot-strap your patch?
I just run it on x86, can't run it on ARM, because Android is not a
posix system, nor a System V compatible system. And my code does not
effect x86.
b) Did you run the testsuite?
Yes, but again my code does not effect x86.
c) When you compare the test results with and without the patch, were there any
regressions?
Only the bug has gone. My app can run on my Android ARM system.
On Fri, Apr 18, 2014 at 12:21:50PM +0800, lin zuojian wrote:
Hi,
Here is the patch after the Jakub's review, and Jakub helps with the
coding style.
--
* asan.c (asan_emit_stack_protection):
Force the base to align to appropriate bits if STRICT_ALIGNMENT. Set
shadow_mem align to appropriate bits if STRICT_ALIGNMENT.
* cfgexpand.c
(expand_stack_vars): Set base_align appropriately when asan is on.
(expand_used_vars): Leave a space in the stack frame for alignment if
STRICT_ALIGNMENT.
---
gcc/ChangeLog | 9 +
gcc/asan.c | 15 +++
gcc/cfgexpand.c | 18 --
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index da35be8..30a2b33 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2014-04-18 Lin Zuojian manjian2...@gmail.com
+ PR middle-end/60281
+ * asan.c (asan_emit_stack_protection): Force the base to align to
+ appropriate bits if STRICT_ALIGNMENT. Set shadow_mem align to
+ appropriate bits if STRICT_ALIGNMENT.
+ * cfgexpand.c (expand_stack_vars): Set base_align appropriately
+ when asan is on.
+ (expand_used_vars): Leave a space in the stack frame for alignment
+ if STRICT_ALIGNMENT.
2014-04-17 Jakub Jelinek ja...@redhat.com
PR target/60847
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..28a476f 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,17 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
+ASAN_SHADOW_SHIFT)
+ / BITS_PER_UNIT), Pmode), NULL_RTX,
+ 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1106,8 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
@@ -1186,6 +1197,10 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+ if (STRICT_ALIGNMENT)
+set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
prev_offset = base_offset;
last_offset = base_offset;
last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index b7f6360..14511e1 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,19 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX (crtl-max_used_stack_slot_alignment,
+ GET_MODE_ALIGNMENT (SImode)
+ASAN_SHADOW_SHIFT);
}
else
- offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ {
+ offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1845,6 +1854,11 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space
Re: [PATCH v7] PR middle-end/60281
Hi, GCC changes(http://gcc.gnu.org/gcc-4.9/changes.html) says: AddressSanitizer, a fast memory error detector, is now available on ARM. But I test it,and find PR60281 is still there!That means the wrong shadow bytes will still populate using stm instructions. So,I don't think asan is available on ARM. Anyway I have signed the FSF copyright form.I hope my patch will be apply on newer GCC. -- Regards lin zuojian
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, Post stations are not that 90's,and they charge.It took me $30 to post the file to USA.It's so inconvenient and expensive that I can't send a scaned version. -- Regards lin zuojian On Wed, Apr 09, 2014 at 06:47:56PM +0800, lin zuojian wrote: Hi Bernd, I am asking them if they would accept a scaned image version.Post station is so 90's -- Regards lin zuojian
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, Seem we are not talking the same problem.You should first make sure what has been going wrong first. And I will sign it. -- Regards lin zuojian
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, I am asking them if they would accept a scaned image version.Post station is so 90's -- Regards lin zuojian
Re: [PATCH v7?] PR middle-end/60281
Hi Bernd, in a way,yes.I am testing it.Since I don't have a FSF copyright mark,and I have no idea what is that file saying,I can't commit my patch.If you are interested in it,please help me commit it. By the way,there is another way to work it out,and it has been mentioned in the previous(I don't know how previous) mail.That is to use unaligned instructions instead of aligned ones on the armv7 or above platform.That has to modify part of machine desc of arm platform.I can't tell you it's one of the define_peephole.I have no time to test this idea yet.If you are interested in that way,please test this one.It's always better when there is an option. -- Regards lin zuojian
Re: [PATCH v7] PR middle-end/60281
On Tue, Mar 04, 2014 at 09:04:56AM +0100, Jakub Jelinek wrote: On Tue, Mar 04, 2014 at 11:11:45AM +0800, lin zuojian wrote: Without aligning the asan stack base,this base will only 64-bit aligned in ARM machines. But asan require 256-bit aligned base because of this: 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros 2.store multiple/load multiple instructions require the other 2 bits are zeros that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits aligned. * asan.c (asan_emit_stack_protection): Force the base to align to appropriate bits if STRICT_ALIGNMENT. Set shadow_mem align to appropriate bits if STRICT_ALIGNMENT. * cfgexpand.c (expand_stack_vars): Set base_align appropriately when asan is on. (expand_used_vars): Leave a space in the stack frame for alignment if STRICT_ALIGNMENT. There were still a couple of formatting issues, I've fixed them below. Now, do you have a GCC copyright assignment or are under copyright assignment of some company working on GCC (ARM?)? Thanks for the fixing!. I have none of them.I have read the website,but don't know what exactly I am expecting to do.
How my codes comply with gcc code formatting?
Hi, I have summited a patch in this ml.But the difficult part may be the code formatting.I am using vim as my editor. -- Regards lin zuojian.
Re: How my codes comply with gcc code formatting?
On Wed, Mar 05, 2014 at 07:28:35AM +0100, Marek Polacek wrote: On Wed, Mar 05, 2014 at 11:31:59AM +0800, lin zuojian wrote: Hi, I have summited a patch in this ml.But the difficult part may be the code formatting.I am using vim as my editor. I'm not sure what you asking for here. But to learn how to format your code, I think just look at the GCC source code and model your code after that. Marek Hi Marek, I am not understanding the standard.For example,should I use \t instead of space?Should I use 4 spaces as indent or 2? -- Regards lin zuojian
Re: How my codes comply with gcc code formatting?
On Wed, Mar 05, 2014 at 11:29:07AM +0400, Yury Gribov wrote: That is not true. The indentation style is: http://www.gnu.org/prep/standards/standards.html#Formatting ... The above also mentions particular options for GNU indent which set various rules. Ah, good to know. So, are contributors expected to run indent on their changes before sending patches? Or maybe add some checks to check_GNU_style.sh? -Y Yeah.Very good if there is such a script.
Re: [PATCH v4] PR middle-end/60281
Hi Jakub,
Any comments on this patch?
--
Regards
lin zuojian
On Tue, Feb 25, 2014 at 03:28:14PM +0800, lin zuojian wrote:
Sorry,I have forgot setting another shadow_mem's align.And many strbs
bump up.
Here 's patch v4.(last one contains html,damn thunderbird).
--
Without aligning the asan stack base,this base will only 64-bit aligned
in ARM machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are
zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256
bits aligned.
* asan.c (asan_emit_stack_protection): Forcing the base to align to 256
bits if STRICT_ALIGNMENT.
And set shadow_mem align to 256 bits if STRICT_ALIGNMENT
* cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is on
(expand_used_vars): Leaving a space in the stack frame for alignment if
STRICT_ALIGNMENT
---
gcc/asan.c | 14 ++
gcc/cfgexpand.c | 13 -
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..64898cd 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,16 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) /
BITS_PER_UNIT), Pmode),
+ NULL_RTX, 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1105,8 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
@@ -1186,6 +1196,10 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
prev_offset = base_offset;
last_offset = base_offset;
last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 06d494c..14fd1c2 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX(crtl-max_used_stack_slot_alignment,
+ (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT));
}
else
+ {
offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,9 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
--
1.8.3.2
Re: [PATCH v4] PR middle-end/60281
Hi Jakub, No. As I wrote earlier, the alternative is to use unaligned stores for ARM, I've asked Lin to benchmark that compared to his patch, but haven't seen that done yet. Jakub I have not benchmark yet.But according to what I hear from an ARM Engineer in Huawei, unaligned accessing usually slow.And not recommand to use too much. -- Regards lin zuojian
Re: [PATCH v4] PR middle-end/60281
Okay,I will use mutt as my MUA. -- Regards lin zuojian On Mon, Mar 03, 2014 at 09:58:59AM +0100, Jakub Jelinek wrote: On Mon, Mar 03, 2014 at 04:51:20PM +0800, lin zuojian wrote: Hi Jakub, Any comments on this patch? Can you please repost the patch (+ ChangeLog entry) as attachment? Or use saner MUA? When all tabs are eaten and other whitespace crippled, it is impossible to look at the formatting. Jakub
Re: [PATCH v4] PR middle-end/60281
Hi Jakub, On Mon, Mar 03, 2014 at 10:08:53AM +0100, Jakub Jelinek wrote: On Mon, Mar 03, 2014 at 05:04:45PM +0800, lin zuojian wrote: No. As I wrote earlier, the alternative is to use unaligned stores for ARM, I've asked Lin to benchmark that compared to his patch, but haven't seen that done yet. I have not benchmark yet.But according to what I hear from an ARM Engineer in Huawei, unaligned accessing usually slow.And not recommand to use too much. It is expected it will not be as fast as aligned store, the question is if an unaligned 32-bit store is faster than 4 8-bit stores, and/or if the cost of the unaligned stores is bad enough (note, usually it is just a few stores in the prologue and epilogue) to offset for the penalties introduced by realigning the stack (typically one extra register that has to be live, plus the cost of the realignment itself). Will I run some microbenchmarks?Like char * a = new char[100]; a++; int * b = reinterpret_castint*(a); b[0] = xxx; b[4] = xxx; b[1] = xxx; b[2] = xxx; b[3] = xxx; ... I don't know if this is accurate. Jakub
[Patch] Try to peephole2 QI *4 into SI on i386
---
gcc/config/i386/i386.md | 49 +
1 file changed, 49 insertions(+)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b9f1320..86ab025 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -18535,6 +18535,55 @@
[(set_attr type other)
(set_attr length 3)])
+(define_peephole2
+ [(set (mem:QI (match_operand 0 register_operand))
+ (match_operand 1 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 1)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 2)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 3)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ int32_t _const = (INTVAL(operands[1]) 0xff) | (((INTVAL(operands[2]))
0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
+(define_peephole2
+ [(set (mem:QI (plus (match_operand 0 register_operand)
+(match_operand 6
const_int_operand)))
+ (match_operand 1 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 7
const_int_operand)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 8
const_int_operand)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 9
const_int_operand)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ if ((INTVAL(operands[7]) - INTVAL(operands[6]) != 1)
+(INTVAL(operands[8]) - INTVAL(operands[7]) != 1)
+(INTVAL(operands[9]) - INTVAL(operands[8]) != 1))
+ FAIL;
+ int32_t _const = (INTVAL(operands[1]) 0xff) | (((INTVAL(operands[2]))
0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
(include mmx.md)
(include sse.md)
(include sync.md)
--
1.8.3.2
- End forwarded message -
[PATCH v5] PR middle-end/60281
Hi,
In this patch I only add ChangeLog.
--
Without aligning the asan stack base,this base will only 64-bit aligned in ARM
machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits
aligned.
* asan.c (asan_emit_stack_protection): Forcing the base to align to
appropriate bits if STRICT_ALIGNMENT.And set shadow_mem align to
appropriate bits if STRICT_ALIGNMENT.
* cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is
on.
(expand_used_vars): Leaving a space in the stack frame for alignment if
STRICT_ALIGNMENT.
Signed-off-by: lin zuojian manjian2...@gmail.com
---
gcc/ChangeLog | 10 ++
gcc/asan.c | 14 ++
gcc/cfgexpand.c | 13 -
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c056946..c00bfc4 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2014-03-03 lin zuojian manjian2...@gmail.com
+ Fix PR middle-end/60281
+ * asan.c (asan_emit_stack_protection): Forcing the base to align to
+ appropriate bits if STRICT_ALIGNMENT.And set shadow_mem align to
+ appropriate bits if STRICT_ALIGNMENT.
+ * cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is
+ on.
+ (expand_used_vars): Leaving a space in the stack frame for alignment if
+ STRICT_ALIGNMENT.
+
2014-03-02 Jan Hubicka hubi...@ucw.cz
PR ipa/60150
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..64898cd 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,16 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT), Pmode),
+ NULL_RTX, 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1105,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned
int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
@@ -1186,6 +1196,10 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
prev_offset = base_offset;
last_offset = base_offset;
last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 06d494c..14fd1c2 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX(crtl-max_used_stack_slot_alignment,
+ (GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT));
}
else
+ {
offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,9 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
--
1.8.3.2
[Patch v2] Try to peephole2 QI *4 into SI on i386
Hi,
Patch v1 is just wrong.
--
Regards
lin zuojian
---
gcc/config/i386/i386.md | 50 +
1 file changed, 50 insertions(+)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b9f1320..e44fb14 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -18535,6 +18535,56 @@
[(set_attr type other)
(set_attr length 3)])
+(define_peephole2
+ [(set (mem:QI (match_operand 0 register_operand))
+ (match_operand 1 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 1)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 2)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 3)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ int32_t _const = (INTVAL(operands[1]) 0xff) | (((INTVAL(operands[2]))
0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
+(define_peephole2
+ [(set (mem:QI (plus (match_operand 0 register_operand)
+(match_operand 6
const_int_operand)))
+ (match_operand 1 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 7
const_int_operand)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 8
const_int_operand)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 9
const_int_operand)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (plus (match_dup 0)
+ (match_dup 6)))
+ (match_operand 5 const_int_operand))]
+{
+ if ((INTVAL(operands[7]) - INTVAL(operands[6]) != 1)
+(INTVAL(operands[8]) - INTVAL(operands[7]) != 1)
+(INTVAL(operands[9]) - INTVAL(operands[8]) != 1))
+ FAIL;
+ int32_t _const = (INTVAL(operands[1]) 0xff) | (((INTVAL(operands[2]))
0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
(include mmx.md)
(include sse.md)
(include sync.md)
--
1.8.3.2
[PATCH v6] PR middle-end/60281
Hi,
This patch fixes wrong formatting as Jakub points out.
--
Regards
lin zuojian
--
Without aligning the asan stack base,this base will only 64-bit aligned in ARM
machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits
aligned.
* asan.c (asan_emit_stack_protection): Force the base to align to
appropriate bits if STRICT_ALIGNMENT. Set shadow_mem align to
appropriate bits if STRICT_ALIGNMENT.
* cfgexpand.c (expand_stack_vars): Set base_align appropriately
when asan is on.
(expand_used_vars): Leave a space in the stack frame for alignment if
STRICT_ALIGNMENT.
Signed-off-by: lin zuojian manjian2...@gmail.com
---
gcc/ChangeLog | 10 ++
gcc/asan.c | 15 +++
gcc/cfgexpand.c | 16 ++--
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6f5bd57..6cd1ba8 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2014-03-03 Lin Zuojian manjian2...@gmail.com
+ PR middle-end/60281
+ * asan.c (asan_emit_stack_protection): Force the base to align to
+ appropriate bits if STRICT_ALIGNMENT. Set shadow_mem align to
+ appropriate bits if STRICT_ALIGNMENT.
+ * cfgexpand.c (expand_stack_vars): Set base_align appropriately
+ when asan is on.
+ (expand_used_vars): Leave a space in the stack frame for alignment if
+ STRICT_ALIGNMENT.
+
2014-03-03 Uros Bizjak ubiz...@gmail.com
* config/i386/xmmintrin.h (enum _mm_hint) _MM_HINT_ET0: Correct
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..28a476f 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,17 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+base = expand_binop (Pmode, and_optab, base,
+gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
+ ASAN_SHADOW_SHIFT)
+/ BITS_PER_UNIT), Pmode), NULL_RTX,
+1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1106,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned
int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
@@ -1186,6 +1197,10 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+ if (STRICT_ALIGNMENT)
+set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
prev_offset = base_offset;
last_offset = base_offset;
last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 5c23b72..c72c30f 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+base_align = crtl-max_used_stack_slot_alignment;
+ else
+base_align = MAX (crtl-max_used_stack_slot_alignment,
+GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT);
}
else
- offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ {
+ offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,10 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
+ ASAN_SHADOW_SHIFT) / BITS_PER_UNIT, 1);
var_end_seq
[Patch v3] Try to peephole2 QI *4 into SI on i386
Hi,
Patch v1 v2 all have problem.Checkout v3.
--
Regards
lin zuojian
---
gcc/config/i386/i386.md | 49 +
1 file changed, 49 insertions(+)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b9f1320..80d3bf7 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -18535,6 +18535,55 @@
[(set_attr type other)
(set_attr length 3)])
+(define_peephole2
+ [(set (mem:QI (match_operand 0 register_operand))
+ (match_operand 1 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 1)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 2)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 3)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ int32_t _const = (INTVAL(operands[1]) 0xff) | (((INTVAL(operands[2]))
0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
+(define_peephole2
+ [(set (mem:QI (plus (match_operand 0 register_operand)
+(match_operand 6
const_int_operand)))
+ (match_operand 1 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 7
const_int_operand)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 8
const_int_operand)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 9
const_int_operand)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ if ((INTVAL(operands[7]) - INTVAL(operands[6]) != 1)
+ || (INTVAL(operands[8]) - INTVAL(operands[7]) != 1)
+ || (INTVAL(operands[9]) - INTVAL(operands[8]) != 1))
+ FAIL;
+ int32_t _const = (INTVAL(operands[1]) 0xff) | (((INTVAL(operands[2]))
0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
(include mmx.md)
(include sse.md)
(include sync.md)
--
1.8.3.2
Re: [Patch] Try to peephole2 QI *4 into SI on i386
On Mon, Mar 03, 2014 at 07:19:51PM -0800, Andrew Pinski wrote:
On Mon, Mar 3, 2014 at 5:02 AM, lin zuojian manjian2...@gmail.com wrote:
Testcase?
The test case is the same with
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23684.
How about making a generic pass which does this?
Maybe GIMPLE should add a pass about it.Now slp pass has hadled it,but
only available in O3.
See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23684 also. At the
same time this can be used to do the store pair optimization for
ARM/AARCH64 too.
Thanks,
Andrew
---
gcc/config/i386/i386.md | 49
+
1 file changed, 49 insertions(+)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b9f1320..86ab025 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -18535,6 +18535,55 @@
[(set_attr type other)
(set_attr length 3)])
+(define_peephole2
+ [(set (mem:QI (match_operand 0 register_operand))
+ (match_operand 1
const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 1)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 2)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 3)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ int32_t _const = (INTVAL(operands[1]) 0xff) |
(((INTVAL(operands[2])) 0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
+(define_peephole2
+ [(set (mem:QI (plus (match_operand 0 register_operand)
+(match_operand 6
const_int_operand)))
+ (match_operand 1
const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 7
const_int_operand)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 8
const_int_operand)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 9
const_int_operand)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ if ((INTVAL(operands[7]) - INTVAL(operands[6]) != 1)
+(INTVAL(operands[8]) - INTVAL(operands[7]) != 1)
+(INTVAL(operands[9]) - INTVAL(operands[8]) != 1))
+ FAIL;
+ int32_t _const = (INTVAL(operands[1]) 0xff) |
(((INTVAL(operands[2])) 0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
(include mmx.md)
(include sse.md)
(include sync.md)
--
1.8.3.2
- End forwarded message -
Re: [Patch] Try to peephole2 QI *4 into SI on i386
And please note that my patch is for the situation where expansion has
been done.Like instrumentation.
--
Regards
lin zuojian
On Tue, Mar 04, 2014 at 11:44:06AM +0800, lin zuojian wrote:
On Mon, Mar 03, 2014 at 07:19:51PM -0800, Andrew Pinski wrote:
On Mon, Mar 3, 2014 at 5:02 AM, lin zuojian manjian2...@gmail.com wrote:
Testcase?
The test case is the same with
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23684.
How about making a generic pass which does this?
Maybe GIMPLE should add a pass about it.Now slp pass has hadled it,but
only available in O3.
See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23684 also. At the
same time this can be used to do the store pair optimization for
ARM/AARCH64 too.
Thanks,
Andrew
---
gcc/config/i386/i386.md | 49
+
1 file changed, 49 insertions(+)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b9f1320..86ab025 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -18535,6 +18535,55 @@
[(set_attr type other)
(set_attr length 3)])
+(define_peephole2
+ [(set (mem:QI (match_operand 0 register_operand))
+ (match_operand 1
const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 1)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 2)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(const_int 3)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ int32_t _const = (INTVAL(operands[1]) 0xff) |
(((INTVAL(operands[2])) 0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
+(define_peephole2
+ [(set (mem:QI (plus (match_operand 0 register_operand)
+(match_operand 6
const_int_operand)))
+ (match_operand 1
const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 7
const_int_operand)))
+ (match_operand 2 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 8
const_int_operand)))
+ (match_operand 3 const_int_operand))
+ (set (mem:QI (plus (match_dup 0)
+(match_operand 9
const_int_operand)))
+ (match_operand 4 const_int_operand))]
+
+ [(set (mem:SI (match_dup 0))
+ (match_operand 5 const_int_operand))]
+{
+ if ((INTVAL(operands[7]) - INTVAL(operands[6]) != 1)
+(INTVAL(operands[8]) - INTVAL(operands[7]) !=
1)
+(INTVAL(operands[9]) - INTVAL(operands[8]) !=
1))
+ FAIL;
+ int32_t _const = (INTVAL(operands[1]) 0xff) |
(((INTVAL(operands[2])) 0xff) 8)
+
| (((INTVAL(operands[3])) 0xff) 16) |
(((INTVAL(operands[4])) 0xff) 24);
+ operands[5] = gen_rtx_CONST_INT (SImode, _const);
+}
+)
+
(include mmx.md)
(include sse.md)
(include sync.md)
--
1.8.3.2
- End forwarded message -
Re: [Patch] Try to peephole2 QI *4 into SI on i386
On Tue, Mar 04, 2014 at 08:48:40AM +0100, Jakub Jelinek wrote: On Mon, Mar 03, 2014 at 07:19:51PM -0800, Andrew Pinski wrote: On Mon, Mar 3, 2014 at 5:02 AM, lin zuojian manjian2...@gmail.com wrote: Testcase? How about making a generic pass which does this? See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=23684 also. At the same time this can be used to do the store pair optimization for ARM/AARCH64 too. Yeah, I'll try to ressurrect my PR22141 patch after 4.9 branches, and would appreciate if more people would cooperate in finding out the best heuristics when it should be applied (for -Os it is usually clear, for non-strict align targets and optimization for speed less so, for strict align targets even less so). Jakub Hi, Seem been handled.Ignore this patch.All of them are wrong. -- Regards lin zuojian
Re: [PATCH v4] PR middle-end/60281
Hi Bernd, I think about whether the best mode is so important to x86_64.There is no mov r/m64,imm64 refering to Intel Software Developer's Manual Volume 2A 3-505.imm32 is the biggest number.And asan clears stack using imms. And even if there is a mov r/m64,imm64 in the future.The gcc peephole pass will do its job.These constant moves will get combined. So Maybe the best mode is not so important. -- Regards lin zuojian
Is LLVM really pulling ahead of gcc?
Hi, I saw http://www.phoronix.com/scan.php?page=articleitem=llvm34_gcc49_compilersnum=1 And LLVM/clang beaten gcc in serval tests.But I ran that tests,e.g.SciMark,it didn't appear to be like this on my i7 machine.Was that article written by Apple? -- Regards lin zuojian
Re: [PATCH v4] PR middle-end/60281
Hi Bernd, set_mem_align is not working like magic. set_mem_align just set the alignment of a memory rtx.And You must aware that you do so because you are sure this rtx IS aligned like this. For arm machines, the base the virtual registers are aligned to 8 bytes.You can't just set_mem_align to get_best_mode(),because this is not the fact.If you do so,the unalign accessing is still there.My realignment code make sure the base of virtual registers is aligned to SImode,and so I can use that fact to set_mem_align. In a word,set_mem_align does not generate realignment code for STRICT_ALIGNMENT machines. As you say that's not the best mode on all machines.But let's fix one bug at a time.You are focusing too much in that mode. So, that is what I mean: this patch makes the stack grow by 32 bytes, just because the emit_stack_protection uses SImode, with unaligned addresses which is not possible for ARM, and not optimal for X86_64. It's because of the realignment,as my comment said.I have to make room for virtual registers's realignment,or they(or the spilled registers) will get damaged by the other subroutine. -- Regards lin zuojian
Re: [PATCH v4] PR middle-end/60281
Hi Bernd, You may send a patch too.Your idea will be more clear. -- Regards lin zuojian On Sun, Mar 02, 2014 at 10:24:52AM +0800, lin zuojian wrote: Hi Bernd, set_mem_align is not working like magic. set_mem_align just set the alignment of a memory rtx.And You must aware that you do so because you are sure this rtx IS aligned like this. For arm machines, the base the virtual registers are aligned to 8 bytes.You can't just set_mem_align to get_best_mode(),because this is not the fact.If you do so,the unalign accessing is still there.My realignment code make sure the base of virtual registers is aligned to SImode,and so I can use that fact to set_mem_align. In a word,set_mem_align does not generate realignment code for STRICT_ALIGNMENT machines. As you say that's not the best mode on all machines.But let's fix one bug at a time.You are focusing too much in that mode. So, that is what I mean: this patch makes the stack grow by 32 bytes, just because the emit_stack_protection uses SImode, with unaligned addresses which is not possible for ARM, and not optimal for X86_64. It's because of the realignment,as my comment said.I have to make room for virtual registers's realignment,or they(or the spilled registers) will get damaged by the other subroutine. -- Regards lin zuojian
Why my mail is not achived
Hi, my mail is not achived by http://gcc.gnu.org/ml/gcc-patches/2014-02/. What's happening? -- Regards lin zuojian
Re: Why my mail is not achived
Hi, I forgot now is Mar.I thought it's Feb.Sorry. -- Regards lin zuojian On Sun, Mar 02, 2014 at 10:57:15AM +0800, lin zuojian wrote: Hi, my mail is not achived by http://gcc.gnu.org/ml/gcc-patches/2014-02/. What's happening? -- Regards lin zuojian
Re: Why my mail is not achived
Yeah, I have realized that. On Sat, Mar 01, 2014 at 10:01:41PM -0500, Patrick Palka wrote: On Sat, Mar 1, 2014 at 9:57 PM, lin zuojian manjian2...@gmail.com wrote: Hi, my mail is not achived by http://gcc.gnu.org/ml/gcc-patches/2014-02/. What's happening? That's last month's archive.
Re: [PATCH v4] PR middle-end/60281
于 2014年02月28日 15:58, lin zuojian 写道: Hi Bernd, I agree you with the mode problem. And I have not change the stack alignment.What I change is the virtual register base's alignment. Realignment must be make in !STRICT_ALIGNMENT machine,or emitting the efficient code is impossible. Sorry, it should be Realignment must be make in STRICT_ALIGNMENT machine. For example 4 set mem:QI X,REG:QI Y will not combine into one set mem:SI X1,REG:SI Y1,if X is not mentioned as SI mode aligned. To make sure X is SI mode algined,virtual register base must be realigned. For this patch,I only intent to make it right.Making it best is next task. -- Regards lin zuojian. 于 2014年02月28日 15:47, Bernd Edlinger 写道: Hi, I see the problem too. But I think it is not necessary to change the stack alignment to solve the problem. It appears to me that the code in asan_emit_stack_protection is just wrong. It uses SImode when the memory is not aligned enough for that mode. This would not happen if that code is rewritten to use get_best_mode, and by the way, even on x86_64 the emitted code is not optimal, because that target could work with DImode more efficiently. So, to fix that, it would be better to concentrate on that function, and use word_mode instead of SImode, and let get_best_mode choose the required mode. Regards Bernd Edlinger.
Re: [PATCH v4] PR middle-end/60281
Hi Bernd, I agree you with the mode problem. And I have not change the stack alignment.What I change is the virtual register base's alignment. Realignment must be make in !STRICT_ALIGNMENT machine,or emitting the efficient code is impossible. For example 4 set mem:QI X,REG:QI Y will not combine into one set mem:SI X1,REG:SI Y1,if X is not mentioned as SI mode aligned. To make sure X is SI mode algined,virtual register base must be realigned. For this patch,I only intent to make it right.Making it best is next task. -- Regards lin zuojian. 于 2014年02月28日 15:47, Bernd Edlinger 写道: Hi, I see the problem too. But I think it is not necessary to change the stack alignment to solve the problem. It appears to me that the code in asan_emit_stack_protection is just wrong. It uses SImode when the memory is not aligned enough for that mode. This would not happen if that code is rewritten to use get_best_mode, and by the way, even on x86_64 the emitted code is not optimal, because that target could work with DImode more efficiently. So, to fix that, it would be better to concentrate on that function, and use word_mode instead of SImode, and let get_best_mode choose the required mode. Regards Bernd Edlinger.
[PATCH v3] PR middle-end/60281
I reviewed the patch, and found a major bug in setting shadow_mem's
alignment.So I release patch v3.
Addition to the bug I mention,a trailing whitespace is also removed.
---
Without aligning the asan stack base,this base will only 64-bit aligned
in ARM machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are
zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256
bits aligned.
* asan.c (asan_emit_stack_protection): Forcing the base to align to 256
bits if STRICT_ALIGNMENT.
And set shadow_mem align to 256 bits if STRICT_ALIGNMENT
* cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is on
(expand_used_vars): Leaving a space in the stack frame for alignment if
STRICT_ALIGNMENT
---
gcc/asan.c | 10 ++
gcc/cfgexpand.c | 13 -
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..4389420 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,16 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) /
BITS_PER_UNIT), Pmode),
+ NULL_RTX, 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1105,8 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 06d494c..14fd1c2 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX(crtl-max_used_stack_slot_alignment,
+ (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT));
}
else
+ {
offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,9 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
--
1.8.3.2
[PATCH v4] PR middle-end/60281
Sorry,I have forgot setting another shadow_mem's align.And many strbs
bump up.
Here 's patch v4.(last one contains html,damn thunderbird).
--
Without aligning the asan stack base,this base will only 64-bit aligned
in ARM machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are
zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256
bits aligned.
* asan.c (asan_emit_stack_protection): Forcing the base to align to 256
bits if STRICT_ALIGNMENT.
And set shadow_mem align to 256 bits if STRICT_ALIGNMENT
* cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is on
(expand_used_vars): Leaving a space in the stack frame for alignment if
STRICT_ALIGNMENT
---
gcc/asan.c | 14 ++
gcc/cfgexpand.c | 13 -
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..64898cd 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,16 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) /
BITS_PER_UNIT), Pmode),
+ NULL_RTX, 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1105,8 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
@@ -1186,6 +1196,10 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+
prev_offset = base_offset;
last_offset = base_offset;
last_size = 0;
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 06d494c..14fd1c2 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX(crtl-max_used_stack_slot_alignment,
+ (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT));
}
else
+ {
offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,9 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
--
1.8.3.2
Re: [PATCH v2] PR middle-end/60281
Hi everyone,
no comments for my patch?
于 2014年02月21日 10:43, lin zuojian 写道:
Hi,
I have misunderstood the meaning of STRICT_ALIGNMENT.Sorry for that Jakub.
I think patch v2 have modified as Jakub suggested.
---
Without aligning the asan stack base,this base will only 64-bit aligned
in ARM machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are
zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256
bits aligned.
* asan.c (asan_emit_stack_protection): Forcing the base to align to 256
bits if STRICT_ALIGNMENT.
And set shadow_mem align to 256 bits if STRICT_ALIGNMENT
* cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is on
(expand_used_vars): Leaving a space in the stack frame for alignment if
STRICT_ALIGNMENT
---
gcc/asan.c | 10 ++
gcc/cfgexpand.c | 13 -
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..8d8ad50 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,16 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) /
BITS_PER_UNIT), Pmode),
+ NULL_RTX, 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1105,8 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 06d494c..63d8020 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX(crtl-max_used_stack_slot_alignment,
+ (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT));
}
else
+ {
offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,9 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
[PATCH] PR middle-end/60281
Without aligning the asan stack base,base will only 64-bit aligned in ARM machines. But asan require 256-bit aligned base because of this: 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros 2.store multiple/load multiple instructions require the other 2 bits are zeros that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits aligned. * asan.c (asan_emit_stack_protection): Forcing the base to align to 256 bits * cfgexpand.c (expand_used_vars): Leaving a space in the stack frame for alignment Signed-off-by: lin zuojian manjian2...@gmail.com --- gcc/asan.c | 6 ++ gcc/cfgexpand.c | 2 ++ 2 files changed, 8 insertions(+) diff --git a/gcc/asan.c b/gcc/asan.c index 53992a8..455e484 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1019,6 +1019,12 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, } if (use_after_return_class == -1 pbase) emit_move_insn (pbase, base); + + /* align base */ + base = expand_binop (Pmode, and_optab, base, + gen_int_mode (-ASAN_RED_ZONE_SIZE, Pmode), + NULL_RTX, 1, OPTAB_DIRECT); + base = expand_binop (Pmode, add_optab, base, gen_int_mode (base_offset - base_align_bias, Pmode), NULL_RTX, 1, OPTAB_DIRECT); diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 06d494c..9e887f7 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -1843,6 +1843,8 @@ expand_used_vars (void) = alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE); data.asan_vec.safe_push (prev_offset); data.asan_vec.safe_push (offset); + /* leave a space for alignment */ + alloc_stack_frame_space (ASAN_RED_ZONE_SIZE, 1); var_end_seq = asan_emit_stack_protection (virtual_stack_vars_rtx, -- 1.8.3.2
Re: [PATCH] PR middle-end/60281
On Thu, Feb 20, 2014 at 04:19:12PM +0800, lin zuojian wrote: Without aligning the asan stack base,base will only 64-bit aligned in ARM machines. But asan require 256-bit aligned base because of this: 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros 2.store multiple/load multiple instructions require the other 2 bits are zeros that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits aligned. * asan.c (asan_emit_stack_protection): Forcing the base to align to 256 bits * cfgexpand.c (expand_used_vars): Leaving a space in the stack frame for alignment This is wrong. You certainly don't want to do any of the extra aligning on non-strict alignment targets, so all the changes must be if (STRICT_ALIGNMENT) guarded, as they are quite expensive (you need one extra register in that case). I have to do realignment.And llvm's backend also do the realignment.I just refers to its implementation. Second thing is, I think you break --param use-after-return=0 this way and also functions with very large stack frames, because emit_move_insn to pbase is then done before you adjust the stack. Yes, that's a defect. Also, I don't think you want to align for ASAN_RED_ZONE_SIZE, but instead (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) / BITS_PER_UNIT, and if you do (of course for STRICT_ALIGNMENT targets only), you should set_mem_align on shadow_mem when it is created. I agree with you on the alignment value.May invoking set_mem_align on shadow_mem happen too late at this point?Given RTL has been expanded. And again I have to do realignment on non STRICT_ALIGNMENT whatever cost it take or there are always alignment faults. If you do the realignment, then it would be nice if the middle-end was told about that, so do base_align = crtl-max_used_stack_slot_alignment; in both the if ((flag_sanitize SANITIZE_ADDRESS) ASAN_STACK pred) then body and else, and if sanitizing and STRICT_ALIGNMENT, set base_align to MIN of crtl-max_used_stack_slot_alignment and the alignment you actually guarantee. Okay,I will do that. Or, if ARM supports unaligned loads/stores using special instructions, perhaps you should also benchmark the alternative of not realigning, but instead making sure those unaligned instructions are used for the shadow memory loads/stores in the asan prologue/epilogue. I have tried to use -fno-peephole2 to shutdown instructions combinations,but that makes me feel uncomfortable. Please next time post the patch from a sane MUA that doesn't eat tabs or at least as an attachment. Comments should start with a capital letter, and with a full stop followed by two spaces, in the ChangeLog also all entries should end with a full stop. Sorry about that... BTW,I didn't modify the ChangeLog because this patch should be discussed carefully. Jakub
Re: [PATCH] PR middle-end/60281
If you don't mind my realigning on STRICT_ALIGNMENT machines,I will make patch v2 soon. On Thu, Feb 20, 2014 at 04:19:12PM +0800, lin zuojian wrote: Without aligning the asan stack base,base will only 64-bit aligned in ARM machines. But asan require 256-bit aligned base because of this: 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros 2.store multiple/load multiple instructions require the other 2 bits are zeros that add up lowest 5 bits should be zeros.That means 32 bytes or 256 bits aligned. * asan.c (asan_emit_stack_protection): Forcing the base to align to 256 bits * cfgexpand.c (expand_used_vars): Leaving a space in the stack frame for alignment This is wrong. You certainly don't want to do any of the extra aligning on non-strict alignment targets, so all the changes must be if (STRICT_ALIGNMENT) guarded, as they are quite expensive (you need one extra register in that case). I have to do realignment.And llvm's backend also do the realignment.I just refers to its implementation. Second thing is, I think you break --param use-after-return=0 this way and also functions with very large stack frames, because emit_move_insn to pbase is then done before you adjust the stack. Yes, that's a defect. Also, I don't think you want to align for ASAN_RED_ZONE_SIZE, but instead (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) / BITS_PER_UNIT, and if you do (of course for STRICT_ALIGNMENT targets only), you should set_mem_align on shadow_mem when it is created. I agree with you on the alignment value.May invoking set_mem_align on shadow_mem happen too late at this point?Given RTL has been expanded. And again I have to do realignment on non STRICT_ALIGNMENT whatever cost it take or there are always alignment faults. If you do the realignment, then it would be nice if the middle-end was told about that, so do base_align = crtl-max_used_stack_slot_alignment; in both the if ((flag_sanitize SANITIZE_ADDRESS) ASAN_STACK pred) then body and else, and if sanitizing and STRICT_ALIGNMENT, set base_align to MIN of crtl-max_used_stack_slot_alignment and the alignment you actually guarantee. Okay,I will do that. Or, if ARM supports unaligned loads/stores using special instructions, perhaps you should also benchmark the alternative of not realigning, but instead making sure those unaligned instructions are used for the shadow memory loads/stores in the asan prologue/epilogue. I have tried to use -fno-peephole2 to shutdown instructions combinations,but that makes me feel uncomfortable. Please next time post the patch from a sane MUA that doesn't eat tabs or at least as an attachment. Comments should start with a capital letter, and with a full stop followed by two spaces, in the ChangeLog also all entries should end with a full stop. Sorry about that... BTW,I didn't modify the ChangeLog because this patch should be discussed carefully. Jakub
Re: [PATCH] PR middle-end/60281
Oh, you may think my patch have not violated STRICT_ALIGNMENT.My patch doesn't realign stack_pointer,but virtual_stack_vars,although they may share the same position mostly. As you can see,the code emitted just make the stack 8 bytes aligned. You mean that for the prologues right now on ARM we emit unaligned store insns during expansion and that they are later peepholed into aligned stores? Yes.Because SImode is aligned to 4 bytes ,and str is considered aligned to 4 bytes on all ARM machine,so gcc doesn't consider itself emitting the unaligned str instructions in the first place.Of course aligned str can be peepholed into aligned str multiple. -- lin zuojian
Re: [PATCH] PR middle-end/60281
于 2014年02月20日 19:05, Ramana Radhakrishnan 写道: Or, if ARM supports unaligned loads/stores using special instructions, perhaps you should also benchmark the alternative of not realigning, but instead making sure those unaligned instructions are used for the shadow memory loads/stores in the asan prologue/epilogue. I have tried to use -fno-peephole2 to shutdown instructions combinations,but that makes me feel uncomfortable. That should not be required. I suspect what you need is a movmisalignsi expander generating unaligned_store/loadsi patterns. Notice that these put out ldr / str instructions but with UNSPECs which means that the peepholers for ldrd / ldm will not catch them. As Jakub says later in this thread ARM is a STRICT_ALIGNMENT target. If the compiler is peepholing unaligned ldr's and str's, that would be a bug and the compiler needs to be fixed to avoid this. regards Ramana That is not a bug of instruction selection.SImode implies that the address is 4 bytes aligned. Please next time post the patch from a sane MUA that doesn't eat tabs or at least as an attachment. Comments should start with a capital letter, and with a full stop followed by two spaces, in the ChangeLog also all entries should end with a full stop. Sorry about that... BTW,I didn't modify the ChangeLog because this patch should be discussed carefully. Jakub
[PATCH v2] PR middle-end/60281
Hi,
I have misunderstood the meaning of STRICT_ALIGNMENT.Sorry for that Jakub.
I think patch v2 have modified as Jakub suggested.
---
Without aligning the asan stack base,this base will only 64-bit aligned
in ARM machines.
But asan require 256-bit aligned base because of this:
1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
2.store multiple/load multiple instructions require the other 2 bits are
zeros
that add up lowest 5 bits should be zeros.That means 32 bytes or 256
bits aligned.
* asan.c (asan_emit_stack_protection): Forcing the base to align to 256
bits if STRICT_ALIGNMENT.
And set shadow_mem align to 256 bits if STRICT_ALIGNMENT
* cfgexpand.c (expand_stack_vars): set base_align appropriately when
asan is on
(expand_used_vars): Leaving a space in the stack frame for alignment if
STRICT_ALIGNMENT
---
gcc/asan.c | 10 ++
gcc/cfgexpand.c | 13 -
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index 53992a8..8d8ad50 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1017,8 +1017,16 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
base_align_bias = ((asan_frame_size + alignb - 1)
~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
}
+ /* Align base if target is STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ base = expand_binop (Pmode, and_optab, base,
+ gen_int_mode (-((GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT) /
BITS_PER_UNIT), Pmode),
+ NULL_RTX, 1, OPTAB_DIRECT);
+
if (use_after_return_class == -1 pbase)
emit_move_insn (pbase, base);
+
+
base = expand_binop (Pmode, add_optab, base,
gen_int_mode (base_offset - base_align_bias, Pmode),
NULL_RTX, 1, OPTAB_DIRECT);
@@ -1097,6 +1105,8 @@ asan_emit_stack_protection (rtx base, rtx pbase,
unsigned int alignb,
(ASAN_RED_ZONE_SIZE ASAN_SHADOW_SHIFT) == 4);
shadow_mem = gen_rtx_MEM (SImode, shadow_base);
set_mem_alias_set (shadow_mem, asan_shadow_set);
+ if (STRICT_ALIGNMENT)
+ set_mem_align(shadow_mem, (GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT));
prev_offset = base_offset;
for (l = length; l; l -= 2)
{
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 06d494c..63d8020 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1013,10 +1013,18 @@ expand_stack_vars (bool (*pred) (size_t), struct
stack_vars_data *data)
if (data-asan_base == NULL)
data-asan_base = gen_reg_rtx (Pmode);
base = data-asan_base;
+
+ if (!STRICT_ALIGNMENT)
+ base_align = crtl-max_used_stack_slot_alignment;
+ else
+ base_align = MAX(crtl-max_used_stack_slot_alignment,
+ (GET_MODE_ALIGNMENT (SImode) ASAN_SHADOW_SHIFT));
}
else
+ {
offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
- base_align = crtl-max_used_stack_slot_alignment;
+ base_align = crtl-max_used_stack_slot_alignment;
+ }
}
else
{
@@ -1843,6 +1851,9 @@ expand_used_vars (void)
= alloc_stack_frame_space (redzonesz, ASAN_RED_ZONE_SIZE);
data.asan_vec.safe_push (prev_offset);
data.asan_vec.safe_push (offset);
+ /* Leave a space for alignment if STRICT_ALIGNMENT. */
+ if (STRICT_ALIGNMENT)
+ alloc_stack_frame_space ((GET_MODE_ALIGNMENT (SImode)
ASAN_SHADOW_SHIFT) / BITS_PER_UNIT , 1);
var_end_seq
= asan_emit_stack_protection (virtual_stack_vars_rtx,
--
1.8.3.2
