| Message ID | bed0c89d-3d0c-5500-d6e2-1db4de6685ec@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Add basic support for prefixed and PC-relative addresses | expand |
On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote: > * config/rs6000/predicates.md (pcrel_address): New > define_predicate. Please put that on one line? > + if (LABEL_REF_P (x)) > + output_asm_label (x); > + else > + output_addr_const (file, x); Why does LABEL_REF need separate handling here? > + if (offset) > + fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "", > + offset); Maybe if (offset) fprintf (file, "%+" PRId64, offset); (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for that then. Oh well). > +rs6000_prefixed_address (rtx addr, machine_mode mode) > + if (GET_CODE (addr) == PLUS) > + { > + HOST_WIDE_INT value, mask; Please declare these where they are defined? > + /* DS instruction (bottom 2 bits must be 0). For 32-bit integers, we > + need to use DS instructions if we are sign-extending the value with > + LWA. For 32-bit floating point, we need DS instructions to load and > + store values to the traditional Altivec registers. */ > + else if (GET_MODE_SIZE (mode) >= 4) > + mask = 3; I don't much like penalising scalar single precision float like this. But, this also hurts unaligned lwz... Do we have statistics on that? Offline, I guess :-) > + /* Return true if we must use a prefixed instruction. */ > + return ((value & ~mask) != value); (value & mask) != 0 Okay with those things frobbed a little, or looked at. Thanks! Segher Segher
On 5/30/19 2:20 PM, Segher Boessenkool wrote: > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote: >> * config/rs6000/predicates.md (pcrel_address): New >> define_predicate. > Please put that on one line? OK. Emacs in ChangeLog and Fill modes seems to set a line length somewhat less than 79. I generally follow what it tells me, but I can fix this. > >> + if (LABEL_REF_P (x)) >> + output_asm_label (x); >> + else >> + output_addr_const (file, x); > Why does LABEL_REF need separate handling here? Mike, please respond? > >> + if (offset) >> + fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "", >> + offset); > Maybe > > if (offset) > fprintf (file, "%+" PRId64, offset); > > (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for > that then. Oh well). Will have a look. > >> +rs6000_prefixed_address (rtx addr, machine_mode mode) >> + if (GET_CODE (addr) == PLUS) >> + { >> + HOST_WIDE_INT value, mask; > Please declare these where they are defined? ok. > >> + /* DS instruction (bottom 2 bits must be 0). For 32-bit integers, we >> + need to use DS instructions if we are sign-extending the value with >> + LWA. For 32-bit floating point, we need DS instructions to load and >> + store values to the traditional Altivec registers. */ >> + else if (GET_MODE_SIZE (mode) >= 4) >> + mask = 3; > I don't much like penalising scalar single precision float like this. > But, this also hurts unaligned lwz... Do we have statistics on that? > Offline, I guess :-) Again, I'll defer to Mike on those details (online or offline). ;-) > >> + /* Return true if we must use a prefixed instruction. */ >> + return ((value & ~mask) != value); > (value & mask) != 0 OK. > > > Okay with those things frobbed a little, or looked at. Thanks! Thanks for the review on your day off! Bill > > > Segher > > > Segher
On Thu, May 30, 2019 at 02:43:49PM -0500, Bill Schmidt wrote: > On 5/30/19 2:20 PM, Segher Boessenkool wrote: > > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote: > >> * config/rs6000/predicates.md (pcrel_address): New > >> define_predicate. > > Please put that on one line? > > OK. Emacs in ChangeLog and Fill modes seems to set a line length > somewhat less than 79. I generally follow what it tells me, but I can > fix this. This would be just 76 ;-) > >> + if (offset) > >> + fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "", > >> + offset); > > Maybe > > > > if (offset) > > fprintf (file, "%+" PRId64, offset); > > > > (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for > > that then. Oh well). > > Will have a look. Very many things in GCC do this, but it's so inelegant :-) It's fine to commit it like this, as I said. But it annoys me every time I see it :-) Segher
On Thu, May 30, 2019 at 02:43:49PM -0500, Bill Schmidt wrote: > On 5/30/19 2:20 PM, Segher Boessenkool wrote: > > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote: > >> * config/rs6000/predicates.md (pcrel_address): New > >> define_predicate. > > Please put that on one line? > > OK. Emacs in ChangeLog and Fill modes seems to set a line length > somewhat less than 79. I generally follow what it tells me, but I can > fix this. > > > >> + if (LABEL_REF_P (x)) > >> + output_asm_label (x); > >> + else > >> + output_addr_const (file, x); > > Why does LABEL_REF need separate handling here? > > Mike, please respond? The current code doesn't need special handling. I don't recall if earlier code did, or I just assumed output_addr_const did not handle LABEL_REF's. > > I don't much like penalising scalar single precision float like this. > > But, this also hurts unaligned lwz... Do we have statistics on that? > > Offline, I guess :-) > > Again, I'll defer to Mike on those details (online or offline). ;-) I have rewritten this section. In particular rather than basing everything off of the mode, I have a now have a more targeted approach that knows which register, mode, and with sign extension requires D/DS/DQ addresses. Of course this only works after register allocation. Before register allocation, we still have to make a best guess based only on the mode.
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md index a578e0f27f7..8ca98299950 100644 --- a/gcc/config/rs6000/predicates.md +++ b/gcc/config/rs6000/predicates.md @@ -1622,6 +1622,45 @@ return GET_CODE (op) == UNSPEC && XINT (op, 1) == UNSPEC_TOCREL; }) +;; Return true if the operand is a pc-relative address. +(define_predicate "pcrel_address" + (match_code "label_ref,symbol_ref,const") +{ + if (!TARGET_PCREL) + return false; + + /* Discard any CONST's. */ + if (GET_CODE (op) == CONST) + op = XEXP (op, 0); + + /* Validate offset. */ + if (GET_CODE (op) == PLUS) + { + rtx op0 = XEXP (op, 0); + rtx op1 = XEXP (op, 1); + + if (!CONST_INT_P (op1) || !SIGNED_34BIT_OFFSET_P (INTVAL (op1), 0)) + return false; + + op = op0; + } + + return LABEL_REF_P (op) || SYMBOL_REF_PCREL_P (op); +}) + +;; Return 1 if op is a prefixed memory operand +(define_predicate "prefixed_mem_operand" + (match_code "mem") +{ + return rs6000_prefixed_address (XEXP (op, 0), GET_MODE (op)); +}) + +;; Return 1 if op is a memory operand that is not a prefixed memory +;; operand. +(define_predicate "non_prefixed_mem_operand" + (and (match_operand 0 "memory_operand") + (not (match_operand 0 "prefixed_mem_operand")))) + ;; Match the first insn (addis) in fusing the combination of addis and loads to ;; GPR registers on power8. (define_predicate "fusion_gpr_addis" diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h index 18ece005a96..feb1250fb8b 100644 --- a/gcc/config/rs6000/rs6000-protos.h +++ b/gcc/config/rs6000/rs6000-protos.h @@ -154,6 +154,7 @@ extern align_flags rs6000_loop_align (rtx); extern void rs6000_split_logical (rtx [], enum rtx_code, bool, bool, bool); extern bool rs6000_pcrel_p (struct function *); extern bool rs6000_fndecl_pcrel_p (const_tree); +extern bool rs6000_prefixed_address (rtx, machine_mode); #endif /* RTX_CODE */ #ifdef TREE_CODE diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index f9ef8e38314..31b86633118 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -21085,6 +21085,34 @@ print_operand_address (FILE *file, rtx x) { if (REG_P (x)) fprintf (file, "0(%s)", reg_names[ REGNO (x) ]); + + /* Is it a pc-relative address? */ + else if (pcrel_address (x, Pmode)) + { + HOST_WIDE_INT offset; + + if (GET_CODE (x) == CONST) + x = XEXP (x, 0); + + if (GET_CODE (x) == PLUS) + { + offset = INTVAL (XEXP (x, 1)); + x = XEXP (x, 0); + } + else + offset = 0; + + if (LABEL_REF_P (x)) + output_asm_label (x); + else + output_addr_const (file, x); + + if (offset) + fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "", + offset); + + fputs ("@pcrel", file); + } else if (SYMBOL_REF_P (x) || GET_CODE (x) == CONST || GET_CODE (x) == LABEL_REF) { @@ -21569,6 +21597,71 @@ rs6000_pltseq_template (rtx *operands, int which) } #endif +/* Helper function to return whether a MODE can do prefixed loads/stores. + VOIDmode is used when we are loading the pc-relative address into a base + register, but we are not using it as part of a memory operation. As modes + add support for prefixed memory, they will be added here. */ + +static bool +mode_supports_prefixed_address_p (machine_mode mode) +{ + return mode == VOIDmode; +} + +/* Function to return true if ADDR is a valid prefixed memory address that uses + mode MODE. */ + +bool +rs6000_prefixed_address (rtx addr, machine_mode mode) +{ + if (!TARGET_PREFIXED_ADDR || !mode_supports_prefixed_address_p (mode)) + return false; + + /* Check for PC-relative addresses. */ + if (pcrel_address (addr, Pmode)) + return true; + + /* Check for prefixed memory addresses that have a large numeric offset, + or an offset that can't be used for a DS/DQ-form memory operation. */ + if (GET_CODE (addr) == PLUS) + { + HOST_WIDE_INT value, mask; + rtx op0 = XEXP (addr, 0); + rtx op1 = XEXP (addr, 1); + + if (!base_reg_operand (op0, Pmode) || !CONST_INT_P (op1)) + return false; + + value = INTVAL (op1); + if (!SIGNED_34BIT_OFFSET_P (value, 0)) + return false; + + /* Offset larger than 16-bits? */ + if (!SIGNED_16BIT_OFFSET_P (value, 0)) + return true; + + /* DQ instruction (bottom 4 bits must be 0) for vectors. */ + if (GET_MODE_SIZE (mode) >= 16) + mask = 15; + + /* DS instruction (bottom 2 bits must be 0). For 32-bit integers, we + need to use DS instructions if we are sign-extending the value with + LWA. For 32-bit floating point, we need DS instructions to load and + store values to the traditional Altivec registers. */ + else if (GET_MODE_SIZE (mode) >= 4) + mask = 3; + + /* QImode/HImode has no restrictions. */ + else + return true; + + /* Return true if we must use a prefixed instruction. */ + return ((value & ~mask) != value); + } + + return false; +} + #if defined (HAVE_GAS_HIDDEN) && !TARGET_MACHO /* Emit an assembler directive to set symbol visibility for DECL to VISIBILITY_TYPE. */ diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 8119c6621d5..34fa36b6ed9 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -2516,3 +2516,10 @@ extern GTY(()) tree rs6000_builtin_decls[RS6000_BUILTIN_COUNT]; IN_RANGE (VALUE, \ -(HOST_WIDE_INT_1 << 33), \ (HOST_WIDE_INT_1 << 33) - 1 - (EXTRA)) + +/* Flag to mark SYMBOL_REF objects to say they are local addresses and are used + in pc-relative addresses. */ +#define SYMBOL_FLAG_PCREL SYMBOL_FLAG_MACH_DEP + +#define SYMBOL_REF_PCREL_P(X) \ + (SYMBOL_REF_P (X) && SYMBOL_REF_FLAGS (X) & SYMBOL_FLAG_PCREL)