Re: [PATCH] Sanitizing the middle-end interface to the back-end for strict alignment
On 8/15/19 9:47 PM, Bernd Edlinger wrote:
> Hi,
>
> this is the split out part from the "Fix not 8-byte aligned ldrd/strd on
> ARMv5 (PR 89544)"
> which is sanitizing the middle-end interface to the back-end for strict
> alignment,
> and a couple of bug-fixes that are necessary to survive boot-strap.
> It is intended to be applied after the PR 89544 fix.
>
> I think it would be possible to change the default implementation of
> STACK_SLOT_ALIGNMENT
> to make all stack variables always naturally aligned instead of doing that
> only
> in assign_parm_setup_stack, but would still like to avoid changing too many
> things
> that do not seem to have a problem. Since this would affect many targets,
> and more
> kinds of variables that may probably not have a strict alignment problem.
> But I am ready to take your advice though.
>
>
> Boot-strapped and reg-tested on x86_64-pc-linux-gnu and arm-linux-gnueabihf
> Is it OK for trunk?
>
>
Hmm, actually the hunk in assign_parm_setup_stack is not only failing
an assertion, but rather a wrong code bug:
I found now a test case that generates silently wrong code and is fixed
by this patch.
$ cat unaligned-argument-3.c
/* { dg-do compile } */
/* { dg-require-effective-target arm_arm_ok } */
/* { dg-options "-marm -mno-unaligned-access -O3" } */
typedef int __attribute__((aligned(1))) s;
void x(char*, s*);
void f(char a, s f)
{
x(&a, &f);
}
/* { dg-final { scan-assembler-times "str\t\[^\\n\]*\\\[sp\\\]" 1 } } */
/* { dg-final { scan-assembler-times "str\t\[^\\n\]*\\\[sp, #3\\\]" 0 } } */
currently with -marm -mno-unaligned-access -O3 we generate:
f:
@ args = 0, pretend = 0, frame = 8
@ frame_needed = 0, uses_anonymous_args = 0
str lr, [sp, #-4]!
sub sp, sp, #12
mov r3, r0
str r1, [sp, #3] <- may trap
add r0, sp, #7
add r1, sp, #3
strbr3, [sp, #7]
bl x
add sp, sp, #12
@ sp needed
ldr pc, [sp], #4
So I would like to add a test case to the patch as attached.
Tested with a cross, that both dg-final fail currently and are fixed
with the other patches applied.
Is it OK for trunk?
Thanks
Bernd.
2019-08-17 Bernd Edlinger
PR middle-end/89544
* gcc.target/arm/unaligned-argument-3.c: New test.
Index: gcc/testsuite/gcc.target/arm/unaligned-argument-3.c
===
--- gcc/testsuite/gcc.target/arm/unaligned-argument-3.c (revision 0)
+++ gcc/testsuite/gcc.target/arm/unaligned-argument-3.c (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arm_ok } */
+/* { dg-options "-marm -mno-unaligned-access -O3" } */
+
+typedef int __attribute__((aligned(1))) s;
+
+void x(char*, s*);
+void f(char a, s f)
+{
+ x(&a, &f);
+}
+
+/* { dg-final { scan-assembler-times "str\t\[^\\n\]*\\\[sp\\\]" 1 } } */
+/* { dg-final { scan-assembler-times "str\t\[^\\n\]*\\\[sp, #3\\\]" 0 } } */
[doc] Adjust links to the FSF's why-not-lgpl article
Committed.
2019-08-17 Gerald Pfeifer
* doc/include/gpl_v3.texi (Copying): Adjust the link to "Why
not LGPL".
Index: doc/include/gpl_v3.texi
===
--- doc/include/gpl_v3.texi (revision 274599)
+++ doc/include/gpl_v3.texi (working copy)
@@ -729,5 +729,5 @@ program into proprietary programs. If your progra
library, you may consider it more useful to permit linking proprietary
applications with the library. If this is what you want to do, use
the GNU Lesser General Public License instead of this License. But
-first, please read @url{https://www.gnu.org/philosophy/why-not-lgpl.html}.
+first, please read @url{https://www.gnu.org/licenses/why-not-lgpl.html}.
@c man end
[doc] Update reference to elm-lang.org quote in ux.texi
Committed.
Gerald
2019-08-17 Gerald Pfeifer
* doc/ux.texi (User Experience Guidelines): Update reference.
Index: doc/ux.texi
===
--- doc/ux.texi (revision 274599)
+++ doc/ux.texi (working copy)
@@ -9,7 +9,7 @@
@cindex guidelines, user experience
To borrow a slogan from
- @uref{https://elm-lang.org/blog/compilers-as-assistants, Elm},
+@uref{https://elm-lang.org/news/compilers-as-assistants, Elm},
@quotation
@strong{Compilers should be assistants, not adversaries.} A compiler should
Re: [PATCH 8/9] ifcvt: Handle swap-style idioms differently.
> I'm still a bit worried about the overlap between the expanded > noce_convert_multiple_sets and cond_move_process_if_block (5/9). > It seems like we're making noce_convert_multiple_set handle most of > the conditional move cases that cond_move_process_if_block can handle. > But like you say, noce_convert_multiple_set is still restricted to > half-diamonds, whereas cond_move_process_if_block can handle full > diamonds. I see your concern and would also agree on that there should be a clearer demarcation of which method can handle what. I'm not really sure I fully understand the distinction between "noce" and "cond_move" anyway, when we emit conditional moves in the "noce" parts. As said before, in my opinion, both noce_convert_multiple and cond_move_process should be unified rather sooner than later. Yet, before and after my suggested patch, noce_convert_multiple_set can handle blocks that cond_move_process cannot, e.g. cmovs that write to one of the registers the condition checked (like min/max idioms) or even cmovs that touch registers that are modified earlier in the block (like in swap idioms). As far as I understand, this is why the additional temporaries have been introduced in the first place - my patch now tries to limit these temporaries to situations where we really need them. I would argue that there is still sufficient difference between them, even though both can now handle constants. Regards Robin
Go patch committed: allocate defer records on the stack
This Go patch Cherry Zhang allocates defer records on the stack. When
a defer is executed at most once in a function body, we can allocate
the defer record for it on the stack instead of on the heap. This
should make defers of this kind (which are very common) faster. This
is a port of https://golang.org/cl/171758 from the gc repo.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 274598)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-838f926c93898767f0337122725a4f52a1335186
+4b47cadf938caadf563f8d0bb3f7111d06f61752
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/runtime.def
===
--- gcc/go/gofrontend/runtime.def (revision 274591)
+++ gcc/go/gofrontend/runtime.def (working copy)
@@ -287,6 +287,10 @@ DEF_GO_RUNTIME(GO, "__go_go", P2(UINTPTR
DEF_GO_RUNTIME(DEFERPROC, "runtime.deferproc", P3(BOOLPTR, UINTPTR, POINTER),
R0())
+// Defer a function, with stack-allocated defer structure.
+DEF_GO_RUNTIME(DEFERPROCSTACK, "runtime.deferprocStack",
+ P4(POINTER, BOOLPTR, UINTPTR, POINTER), R0())
+
// Convert an empty interface to an empty interface, returning ok.
DEF_GO_RUNTIME(IFACEE2E2, "runtime.ifaceE2E2", P1(EFACE), R2(EFACE, BOOL))
Index: gcc/go/gofrontend/statements.cc
===
--- gcc/go/gofrontend/statements.cc (revision 274169)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -2614,7 +2614,11 @@ Thunk_statement::simplify_statement(Gogo
if (this->classification() == STATEMENT_GO)
s = Statement::make_go_statement(call, location);
else if (this->classification() == STATEMENT_DEFER)
-s = Statement::make_defer_statement(call, location);
+{
+ s = Statement::make_defer_statement(call, location);
+ if ((Node::make_node(this)->encoding() & ESCAPE_MASK) ==
Node::ESCAPE_NONE)
+s->defer_statement()->set_on_stack();
+}
else
go_unreachable();
@@ -3019,13 +3023,45 @@ Defer_statement::do_get_backend(Translat
Location loc = this->location();
Expression* ds = context->function()->func_value()->defer_stack(loc);
- Expression* call = Runtime::make_call(Runtime::DEFERPROC, loc, 3,
- ds, fn, arg);
+ Expression* call;
+ if (this->on_stack_)
+{
+ if (context->gogo()->debug_optimization())
+go_debug(loc, "stack allocated defer");
+
+ Type* defer_type = Defer_statement::defer_struct_type();
+ Expression* defer = Expression::make_allocation(defer_type, loc);
+ defer->allocation_expression()->set_allocate_on_stack();
+ defer->allocation_expression()->set_no_zero();
+ call = Runtime::make_call(Runtime::DEFERPROCSTACK, loc, 4,
+defer, ds, fn, arg);
+}
+ else
+call = Runtime::make_call(Runtime::DEFERPROC, loc, 3,
+ ds, fn, arg);
Bexpression* bcall = call->get_backend(context);
Bfunction* bfunction = context->function()->func_value()->get_decl();
return context->backend()->expression_statement(bfunction, bcall);
}
+Type*
+Defer_statement::defer_struct_type()
+{
+ Type* ptr_type = Type::make_pointer_type(Type::make_void_type());
+ Type* uintptr_type = Type::lookup_integer_type("uintptr");
+ Type* bool_type = Type::make_boolean_type();
+ return Type::make_builtin_struct_type(9,
+"link", ptr_type,
+"frame", ptr_type,
+"panicStack", ptr_type,
+"_panic", ptr_type,
+"pfn", uintptr_type,
+"arg", ptr_type,
+"retaddr", uintptr_type,
+"makefunccanrecover", bool_type,
+"heap", bool_type);
+}
+
// Dump the AST representation for defer statement.
void
Index: gcc/go/gofrontend/statements.h
===
--- gcc/go/gofrontend/statements.h (revision 274169)
+++ gcc/go/gofrontend/statements.h (working copy)
@@ -24,6 +24,7 @@ class Expression_statement;
class Block_statement;
class Return_statement;
class Thunk_statement;
+class Defer_statement;
class Goto_statement;
class Goto_unnamed_statement;
class Label_statement;
@@ -403,6 +404,11 @@ class Statement
Thunk_statement*
thunk_statement();
+ // If this is a defer statement, return it. Otherwise return NULL.
+ Defer_statement*
+ defer_statement()
+ { return this->convert(); }
+
Go patch committed: Support new numeric literal syntax
This Go frontend patch adds support for new numeric literal syntax
that was added to the Go 1.13 release
(https://tip.golang.org/doc/go1.13#language). This supports 0b and 0o
prefixes, and hex floats. This was tested against test/literal2.go in
the gc repo. Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 274613)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-4b47cadf938caadf563f8d0bb3f7111d06f61752
+85857977230437f2b3dcbeea009efbb8b2789039
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/lex.cc
===
--- gcc/go/gofrontend/lex.cc(revision 274169)
+++ gcc/go/gofrontend/lex.cc(working copy)
@@ -986,6 +986,26 @@ Lex::is_hex_digit(char c)
|| (c >= 'a' && c <= 'f'));
}
+// Return whether C is a valid digit in BASE.
+
+bool
+Lex::is_base_digit(int base, char c)
+{
+ switch (base)
+{
+case 2:
+ return c == '0' || c == '1';
+case 8:
+ return c >= '0' && c <= '7';
+case 10:
+ return c >= '0' && c <= '9';
+case 16:
+ return Lex::is_hex_digit(c);
+default:
+ go_unreachable();
+}
+}
+
// not a hex value
#define NHV 100
@@ -1032,13 +1052,24 @@ Lex::hex_val(char c)
return hex_value_lookup_table[static_cast(c)];
}
-// Return whether an exponent could start at P.
+// Return whether an exponent could start at P, in base BASE.
bool
-Lex::could_be_exponent(const char* p, const char* pend)
+Lex::could_be_exponent(int base, const char* p, const char* pend)
{
- if (*p != 'e' && *p != 'E')
-return false;
+ switch (base)
+{
+case 10:
+ if (*p != 'e' && *p != 'E')
+ return false;
+ break;
+case 16:
+ if (*p != 'p' && *p != 'P')
+ return false;
+ break;
+default:
+ go_unreachable();
+}
++p;
if (p >= pend)
return false;
@@ -1062,87 +1093,160 @@ Lex::gather_number()
Location location = this->location();
- bool neg = false;
- if (*p == '+')
-++p;
- else if (*p == '-')
-{
- ++p;
- neg = true;
-}
-
- const char* pnum = p;
+ int base = 10;
+ std::string num;
if (*p == '0')
{
- int base;
- if ((p[1] == 'x' || p[1] == 'X')
- && Lex::is_hex_digit(p[2]))
+ int basecheck;
+ int off;
+ if (p[1] == 'x' || p[1] == 'X')
{
base = 16;
- p += 2;
- pnum = p;
- while (p < pend)
- {
- if (!Lex::is_hex_digit(*p))
- break;
- ++p;
- }
+ basecheck = 16;
+ off = 2;
+ }
+ else if (p[1] == 'o' || p[1] == 'O')
+ {
+ base = 8;
+ basecheck = 8;
+ off = 2;
+ }
+ else if (p[1] == 'b' || p[1] == 'B')
+ {
+ base = 2;
+ basecheck = 2;
+ off = 2;
}
else
{
+ // Old style octal literal. May also be the start of a
+ // floating-point number (e.g., 09.2, 09e2) or an imaginary
+ // literal (e.g., 09i), so we have to accept decimal digits.
base = 8;
- pnum = p;
- while (p < pend)
- {
- if (*p < '0' || *p > '9')
- break;
- ++p;
- }
+ basecheck = 10;
+ off = 0;
+ }
+
+ p += off;
+ if (*p == '_' && Lex::is_base_digit(basecheck, p[1]))
+ ++p;
+
+ while (Lex::is_base_digit(basecheck, *p))
+ {
+ num.push_back(*p);
+ ++p;
+ if (*p == '_' && Lex::is_base_digit(basecheck, p[1]))
+ ++p;
+ }
+
+ // We must see at least one valid digit, except for a case like
+ // 0x.0p1.
+ if (num.length() == 0 && (base != 16 || *p != '.'))
+ {
+ go_error_at(this->location(), "invalid numeric literal");
+ this->lineoff_ = p - this->linebuf_;
+ mpz_t val;
+ mpz_init_set_ui(val, 0);
+ Token ret = Token::make_integer_token(val, location);
+ mpz_clear(val);
+ return ret;
+ }
+
+ bool is_float = false;
+ // A number that looks like an old-style octal literal might
+ // actually be the beginning of a floating-point or imaginary
+ // literal, in which case the value is decimal digits. Handle
+ // that case below by treating the leading '0' as decimal.
+ if (off == 0
+ && (*p == '.' || *p == 'i' || Lex::could_be_exponent(10, p, pend)))
+ {
+ is_float = true;
+ base = 10;
}
+ else if (base == 16
+ && (*p == '.' || Lex::could_be_exponent(16, p, pend)))
+ is_float = true;
- // A partial token that looks like an octal literal might actually
