Folks,

> On 22 Feb 2022, at 14:44, Vladimir Makarov <vmaka...@redhat.com> wrote:
> 
> 
> On 2022-02-20 12:34, Iain Sandoe wrote:
>> 
>> ^^^ this is mostly for my education - the stuff below is a potential 
>> solution to leaving lra-constraints unchanged and fixing the Darwin bug….
>> 
> I'd be really glad if you do manage to fix this w/o changing LRA. Richard has 
> a legitimate point that my proposed change in LRA prohibiting 
> `...;reg=low_sum; ...mem[reg]` might force LRA to generate less optimized 
> code or even might make LRA to generate unrecognized insns `reg = orginal 
> addr` for some ports requiring further fixes in machine-dependent code of the 
> ports.

I think this is within my remit to push without further review - however I’d 
very much welcome any comment you folks have: I’d like to push this before my 
weekly Darwin test run - which is usually started just after the daily bump on 
Saturday morning.

The other RS6000 changes remain, as Vlad pointed out we were not being picky 
enough there - despite getting away with it for longer than I’ve been on the 
project ;)

I tested that the patch fixes the problem on 11.2 (for the testcases provided, 
the bug is latent on master) and causes no regressions on powerpc-darwin9 
(master).

cheers
Iain


[PATCH] LRA, rs6000, Darwin: Revise lo_sum use for forced constants  [PR104117].

Follow up discussion to the initial patch for this PR identified that it is
preferable to avoid the LRA change, and arrange for the target to reject the
hi and lo_sum selections when presented with an invalid address.

We split the Darwin high/low selectors into two:
 1. One that handles non-PIC addresses (kernel mode, mdynamic-no-pic).
 2. One that handles PIC addresses and rejects SYMBOL_REFs unless they are
    suitably wrapped in the MACHOPIC_OFFSET unspec.

The second case is handled by providing a new predicate (macho_pic_address)
that checks the requirements.

Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>

        PR target/PR104117

gcc/ChangeLog:

        * config/rs6000/darwin.md (@machopic_high_<mode>): New.
        (@machopic_low_<mode>): New.
        * config/rs6000/predicates.md (macho_pic_address): New.
        * config/rs6000/rs6000.cc (rs6000_legitimize_address): Do not
        apply the TLS processing to Darwin.
        * lra-constraints.cc (process_address_1): Revert the changes
        in r12-7209.
---
 gcc/config/rs6000/darwin.md     | 19 +++++++++++++++----
 gcc/config/rs6000/predicates.md | 14 ++++++++++++++
 gcc/config/rs6000/rs6000.cc     |  2 +-
 gcc/lra-constraints.cc          | 17 +++++++++++++++--
 4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/gcc/config/rs6000/darwin.md b/gcc/config/rs6000/darwin.md
index 8443585df00..e73d59e8066 100644
--- a/gcc/config/rs6000/darwin.md
+++ b/gcc/config/rs6000/darwin.md
@@ -121,21 +121,32 @@ You should have received a copy of the GNU General Public 
License
    stw %0,lo16(%2)(%1)"
   [(set_attr "type" "store")])
 
-;; 64-bit MachO load/store support
-
 ;; Mach-O PIC.
 
 (define_insn "@macho_high_<mode>"
   [(set (match_operand:P 0 "gpc_reg_operand" "=b*r")
        (high:P (match_operand 1 "" "")))]
-  "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN)"
+  "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && !flag_pic"
   "lis %0,ha16(%1)")
 
 (define_insn "@macho_low_<mode>"
   [(set (match_operand:P 0 "gpc_reg_operand" "=r")
        (lo_sum:P (match_operand:P 1 "gpc_reg_operand" "b")
                   (match_operand 2 "" "")))]
-   "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN)"
+   "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && !flag_pic"
+   "la %0,lo16(%2)(%1)")
+
+(define_insn "@machopic_high_<mode>"
+  [(set (match_operand:P 0 "gpc_reg_operand" "=b*r")
+       (high:P (match_operand 1 "macho_pic_address" "")))]
+  "TARGET_MACHO && flag_pic"
+  "lis %0,ha16(%1)")
+
+(define_insn "@machopic_low_<mode>"
+  [(set (match_operand:P 0 "gpc_reg_operand" "=r")
+       (lo_sum:P (match_operand:P 1 "gpc_reg_operand" "b")
+                  (match_operand 2 "macho_pic_address" "")))]
+   "TARGET_MACHO && flag_pic"
    "la %0,lo16(%2)(%1)")
 
 (define_split
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index c65dfb91f3d..28f6e9883cb 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -2045,3 +2045,17 @@
  (if_then_else (match_test "TARGET_VSX")
   (match_operand 0 "reg_or_cint_operand")
   (match_operand 0 "const_int_operand")))
+
+;; Return true if the operand is a valid Mach-O pic address.
+;;
+(define_predicate "macho_pic_address"
+  (match_code "const,unspec")
+{
+  if (GET_CODE (op) == CONST)
+    op = XEXP (op, 0);
+
+  if (GET_CODE (op) == UNSPEC && XINT (op, 1) == UNSPEC_MACHOPIC_OFFSET)
+    return CONSTANT_P (XVECEXP (op, 0, 0));
+  else
+    return false;
+})
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index a855e8c4c72..9dbab1fc644 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -9028,7 +9028,7 @@ rs6000_legitimize_address (rtx x, rtx oldx 
ATTRIBUTE_UNUSED,
       else
        return force_reg (Pmode, x);
     }
-  if (SYMBOL_REF_P (x))
+  if (SYMBOL_REF_P (x) && !TARGET_MACHO)
     {
       enum tls_model model = SYMBOL_REF_TLS_MODEL (x);
       if (model != 0)
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index b2c4590153c..90b2c56d245 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -3625,8 +3625,21 @@ process_address_1 (int nop, bool check_only_p,
                  *ad.inner = gen_rtx_LO_SUM (Pmode, new_reg, addr);
                  if (!valid_address_p (op, &ad, cn))
                    {
-                     *ad.inner = addr; /* Punt.  */
-                     code = -1;
+                     /* Try to put lo_sum into register.  */
+                     insn = emit_insn (gen_rtx_SET
+                                       (new_reg,
+                                        gen_rtx_LO_SUM (Pmode, new_reg, 
addr)));
+                     code = recog_memoized (insn);
+                     if (code >= 0)
+                       {
+                         *ad.inner = new_reg;
+                         if (!valid_address_p (op, &ad, cn))
+                           {
+                             *ad.inner = addr;
+                             code = -1;
+                           }
+                       }
+
                    }
                }
              if (code < 0)
-- 

Reply via email to