From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2049) id 04A86385783B; Tue, 21 Sep 2021 09:14:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 04A86385783B Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Matthew Malcomson To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/vendors/ARM/heads/morello)] Alternate method to set LSB on labels X-Act-Checkin: gcc X-Git-Author: Matthew Malcomson X-Git-Refname: refs/vendors/ARM/heads/morello X-Git-Oldrev: 9cb745fbe16848eb1b4a47dbc6d4f08ac325ef92 X-Git-Newrev: 95c8fab042e81ebe6aeb31205019752d83a6d8e2 Message-Id: <20210921091451.04A86385783B@sourceware.org> Date: Tue, 21 Sep 2021 09:14:51 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Sep 2021 09:14:51 -0000 https://gcc.gnu.org/g:95c8fab042e81ebe6aeb31205019752d83a6d8e2 commit 95c8fab042e81ebe6aeb31205019752d83a6d8e2 Author: Matthew Malcomson Date: Thu Sep 16 11:33:53 2021 +0100 Alternate method to set LSB on labels We needed to add one to these labels when loading directly as well as when storing in data. The obvious way to set this LSB when loading directly was to put the increment as a hook in the expand pass. This affects the approach when storing in data too. Hence we remove the code we've added to include this LSB when storing as a constant and add the new method that accounts for storing as a constant and loading directly. Making this adjustment in the expand pass did mean we needed to add a new target hook. We call it `TARGET_ADJUST_LABEL_EXPANSION` and call it when `expand_expr_real_1` is expanding a `LABEL_REF`. With this new approach, `aarch64_load_symref_appropriately` now can often be requested to load something of the form `(const (pointer_plus (label_ref) (const_int)))`. Unlike the other forms `aarch64_load_symref_appropriately` gets called with, this RTX is not one that can be shared between insns. (N.b. the same form with a `symbol_ref` can be shared according to `shared_const_p`). `aarch64_load_symref_appropriately` was originally written to use the same immediate RTX expression it gets given in HIGH and LO_SUM expressions. When `aarch64_load_symref_appropriately` is given an RTX of the new form we need to ensure that we do not share that exact RTX between different insns. This would cause an ICE when verifying that no RTL is invalidly shared. We fix that by using the result of `copy_rtx` in some of the insn emitting calls of aarch64_load_symref_appropriately. `copy_rtx` already handles checking for whether we have a SYMBOL_REF or LABEL_REF and avoiding the actual copy for a SYMBOL_REF. Our test checks for both directly getting a label's value and for storing a label's value in the data section. This will provide some test coverage until we can build and run programs on a model. Since this change is essential to be able to jump to a label without changing execution mode building and running programs will provide good coverage of this feature from then on. Diff: --- gcc/config/aarch64/aarch64.c | 54 ++++++++++++++-------- gcc/doc/tm.texi | 11 +++++ gcc/doc/tm.texi.in | 2 + gcc/expr.c | 1 + gcc/target.def | 14 ++++++ .../morello/label-addressing-includes-lsb.c | 32 +++++++++++++ 6 files changed, 96 insertions(+), 18 deletions(-) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index e19a99ad206..864c14dee25 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -3126,6 +3126,12 @@ static void aarch64_load_symref_appropriately (rtx dest, rtx imm, enum aarch64_symbol_type type) { + /* Can not share an RTX of the form ({pointer_,}plus (label_ref) (const_int)), + but can share RTX of the form ({pointer_,}plus (symbol_ref) (const_int)). + This is very rarely needed, but it is at least triggered when adding the + LSB to a label_ref for Morello. Checking whether we actually need to do + the copy or not is done inside `copy_rtx`. */ + rtx imm2 = copy_rtx (imm); /* Assert that any time we're asked to load an indirection symbol there is no offset. If there were an offset that would break one of the assumptions we have to believe that loading symbols is safe. (I.e. if we're ever @@ -3148,7 +3154,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, if (can_create_pseudo_p ()) tmp_reg = gen_reg_rtx (mode); - emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm)); + emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm2)); check_emit_insn (gen_add_losym (dest, tmp_reg, imm)); return; } @@ -3245,7 +3251,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, if (can_create_pseudo_p ()) tmp_reg = gen_reg_rtx (mode); - emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm)); + emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm2)); if (mode == ptr_mode) { insn = gen_ldr_got_small (mode, dest, tmp_reg, imm); @@ -3320,7 +3326,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, check_emit_insn (gen_rtx_SET (dest, gen_raw_pointer_plus (sa, tp, x0))); if (REG_P (dest)) - set_unique_reg_note (get_last_insn (), REG_EQUIV, imm); + set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2); return; } @@ -3355,7 +3361,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, check_emit_insn (gen_rtx_SET (dest, gen_raw_pointer_plus (sa, tp, om_reg))); if (REG_P (dest)) - set_unique_reg_note (get_last_insn (), REG_EQUIV, imm); + set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2); return; } @@ -3393,7 +3399,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, } if (REG_P (dest)) - set_unique_reg_note (get_last_insn (), REG_EQUIV, imm); + set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2); return; } @@ -3432,7 +3438,7 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, } if (REG_P (dest)) - set_unique_reg_note (get_last_insn (), REG_EQUIV, imm); + set_unique_reg_note (get_last_insn (), REG_EQUIV, imm2); return; } @@ -19888,18 +19894,15 @@ aarch64_asm_output_capability (rtx x, unsigned int size, int aligned_p) if (TARGET_CAPABILITY_FAKE) return targetm.asm_out.integer(address_value, size, aligned_p); - if (SYMBOL_REF_P (cap_val) - && SUBREG_P (address_value) - && subreg_lowpart_p (address_value) - && LABEL_REF_P (SUBREG_REG (address_value))) - { - /* For a pointer to a label, we need to set the LSB to preserve - PSTATE.C64 for purecap. */ - if (TARGET_CAPABILITY_PURE) - XEXP (x, 1) = plus_constant (POmode, address_value, 1); - } - else - gcc_unreachable (); + gcc_assert (SYMBOL_REF_P (cap_val) + && SUBREG_P (address_value) + && subreg_lowpart_p (address_value)); + rtx sub = SUBREG_REG (address_value); + gcc_assert ((TARGET_CAPABILITY_PURE + && GET_CODE (sub) == CONST + && POINTER_PLUS_P (XEXP (sub, 0)) + && LABEL_REF_P (XEXP (XEXP (sub, 0), 0))) + || (!TARGET_CAPABILITY_PURE && LABEL_REF_P (sub))); } /* Fake capability => size is the correct size and just want to emit the @@ -24272,6 +24275,18 @@ aarch64_target_capability_mode () return opt_scalar_addr_mode (); } +/* Implement TARGET_ADJUST_LABEL_EXPANSION hook. */ +rtx +aarch64_adjust_label_expansion (rtx x) +{ + /* For a pointer to a label, we need to set the LSB to preserve PSTATE.C64 + for purecap (jumps to a register will take PSTATE.C64 from the LSB set in + that register). */ + if (TARGET_CAPABILITY_PURE) + return plus_constant (GET_MODE (x), x, 1); + return x; +} + /* Target-specific selftests. */ #if CHECKING_P @@ -24840,6 +24855,9 @@ aarch64_libgcc_floating_mode_supported_p #undef TARGET_CAPABILITY_MODE #define TARGET_CAPABILITY_MODE aarch64_target_capability_mode +#undef TARGET_ADJUST_LABEL_EXPANSION +#define TARGET_ADJUST_LABEL_EXPANSION aarch64_adjust_label_expansion + #undef TARGET_ASM_DECLARE_CONSTANT_NAME #define TARGET_ASM_DECLARE_CONSTANT_NAME aarch64_declare_constant_name diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 56fe193f363..982f51bcfc4 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -8207,6 +8207,17 @@ Even if the current target does not use the REPLACE_ADDRESS_VALUE RTL code this function will be given that code, so it must be handled by this hook. @end deftypefn +@deftypefn {Target Hook} rtx TARGET_ADJUST_LABEL_EXPANSION (rtx @var{x}) +Adjusts the RTX that has come from expanding a label. This RTX has come +from an expansion of a label and is to be used as data (e.g. in a computed +goto) rather than to be jumped to directly. This hook should take @var{x} +and return a new RTX adjusted with any machine-specific requirements. + +Most targets do not need to implement this hook. The default returns +@var{x} directly. One target that does need to implement it is AArch64. +AArch64 needs to set the LSB of labels depending on the processor state. +@end deftypefn + @deftypefn {Target Hook} void TARGET_ASM_DECL_END (void) Define this hook if the target assembler requires a special marker to terminate an initialized variable declaration. diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 6d037ef77e1..067be34c62c 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -5226,6 +5226,8 @@ It must not be modified by command-line option processing. @hook TARGET_ASM_CAPABILITY +@hook TARGET_ADJUST_LABEL_EXPANSION + @hook TARGET_ASM_DECL_END @hook TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA diff --git a/gcc/expr.c b/gcc/expr.c index 3c682e0584c..a04b77f6309 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -10269,6 +10269,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode tmode, && function != 0) LABEL_REF_NONLOCAL_P (temp) = 1; + temp = targetm.adjust_label_expansion (temp); temp = gen_rtx_MEM (FUNCTION_MODE, temp); return temp; } diff --git a/gcc/target.def b/gcc/target.def index d20db1da93e..2571c5583c0 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -3254,6 +3254,20 @@ DEFHOOK opt_scalar_addr_mode. This is the default.", opt_scalar_addr_mode, (), default_capability_mode) +/* Machine specific adjustments when expanding a label. */ +DEFHOOK +(adjust_label_expansion, + "Adjusts the RTX that has come from expanding a label. This RTX has come\n\ +from an expansion of a label and is to be used as data (e.g. in a computed\n\ +goto) rather than to be jumped to directly. This hook should take @var{x}\n\ +and return a new RTX adjusted with any machine-specific requirements.\n\ +\n\ +Most targets do not need to implement this hook. The default returns\n\ +@var{x} directly. One target that does need to implement it is AArch64.\n\ +AArch64 needs to set the LSB of labels depending on the processor state.", + rtx, (rtx x), + hook_rtx_rtx_identity) + /* Support for named address spaces. */ #undef HOOK_PREFIX #define HOOK_PREFIX "TARGET_ADDR_SPACE_" diff --git a/gcc/testsuite/gcc.target/aarch64/morello/label-addressing-includes-lsb.c b/gcc/testsuite/gcc.target/aarch64/morello/label-addressing-includes-lsb.c new file mode 100644 index 00000000000..2c4a9b27d13 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/morello/label-addressing-includes-lsb.c @@ -0,0 +1,32 @@ +/* { dg-do compile } */ +/* This testcase ensures: + 1) When a label is stored in the constant pool, it is stored using its + containing function's permissions and with the LSB set. + 2) When a label is loaded directly in code (using adrp and add on the label + symbol directly) it has its LSB set. */ +int fun(int x) +{ + void *a[] = {&&label, &&label2}; + label: + goto *a[x++]; + label2: + return x; +} +void* fun2(int x) +{ + void *a = &&label; +label: + if (x % 10) + return a; + else + { +label2: + return &&label2; /* { dg-warning "returns address of label" } */ + } +} + +/* Ensure that we initialise labels in the constant pool with the correct form. */ +/* { dg-final { scan-assembler {capinit\tfun\+\(\(\.L\d\+1\)-fun\)} { target cheri_capability_pure } } } */ + +/* Ensure that we load labels directly with the LSB set. */ +/* { dg-final { scan-assembler {adrp[^\n]*\.L\d\+1} { target cheri_capability_pure } } } */