From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ubizjak@gmail.com>
Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com
 [IPv6:2607:f8b0:4864:20::f31])
 by sourceware.org (Postfix) with ESMTPS id 25F513857C65
 for <gcc-patches@gcc.gnu.org>; Tue, 11 Aug 2020 09:56:16 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 25F513857C65
Received: by mail-qv1-xf31.google.com with SMTP id o2so5637651qvk.6
 for <gcc-patches@gcc.gnu.org>; Tue, 11 Aug 2020 02:56:16 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=LZT+6pystkCHEOWS7iIXJbSxIoCmWtR3t4djFBzst2s=;
 b=e5RhMyPm+xQCKVnsp3tRjw0aeD4YrrtAE0yKJ+sijhhzTwajN8bHkMGBHj7fuM735c
 Uq5LtRybllKikN3LPQjvU+8tSahkSyxVPKnE0WyB+9w+XgDNHW9JLcMQrCac7hvGN/1p
 2iWrkVA0LYaWSIfPAFwdwbSgK5gK3kQ9RQ9Fahp9r7qkAaB7wDQPTmwZYbc2VP7OTElL
 Fysp2l5z13Nr2otEVBm7yIpwH3pE8YTzB9WumCvia2zHB35R9TyxU+/wmCQ9deOqMdwz
 YixzPZ5FwpwOMES1/2/FzfH6KWouTbCrV1a4RHRT1ciFtSN4p5WAKQuOKs1AFEKOGHIn
 gl8Q==
X-Gm-Message-State: AOAM5330TaTPBOnVAnQIstzgfiiQznelCFdxmcjFuT/FyWPDG2/je7x8
 Mv7K8ffhwDeTZvMQc8UG0A3WmrR++BUjNGujjFc=
X-Google-Smtp-Source: ABdhPJxtec7xW42Jn1BRpB2QZ34oJ8UC6WSddUI3Gf+iKkRPe4YS0rqKGXX1+vlzpn3dX/bsbXkRHtcxJAZTuyFfwV0=
X-Received: by 2002:ad4:41c9:: with SMTP id a9mr260734qvq.171.1597139775665;
 Tue, 11 Aug 2020 02:56:15 -0700 (PDT)
MIME-Version: 1.0
References: <CAMZc-bzr3Oq93Np-Vc9n2mR_mjO_bT6gBZFB-irNdqwnLc_kWA@mail.gmail.com>
 <CAFULd4bgHy9qOnpu7imYojPn1JjduLmr3hqOskAQExvUN6vqdg@mail.gmail.com>
 <CAMZc-bxijnO40pdt6OXejKg89_SuVtM4evFE1F4hvBt81Dwi5Q@mail.gmail.com>
In-Reply-To: <CAMZc-bxijnO40pdt6OXejKg89_SuVtM4evFE1F4hvBt81Dwi5Q@mail.gmail.com>
From: Uros Bizjak <ubizjak@gmail.com>
Date: Tue, 11 Aug 2020 11:56:04 +0200
Message-ID: <CAFULd4Y7VZjeZ_9z3LQUzZa_8WKxV76+zO3MbJ2HTvmdaeRYnA@mail.gmail.com>
Subject: Re: [PATCH] [PR target/96350]Force ENDBR immediate into memory to
 avoid fake ENDBR opcode.
To: Hongtao Liu <crazylht@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, "H. J. Lu" <hjl.tools@gmail.com>
Content-Type: text/plain; charset="UTF-8"
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00, DKIM_SIGNED,
 DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,
 SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on
 server2.sourceware.org
X-BeenThere: gcc-patches@gcc.gnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org>
List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>,
 <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe>
List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/>
List-Post: <mailto:gcc-patches@gcc.gnu.org>
List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help>
List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>,
 <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe>
X-List-Received-Date: Tue, 11 Aug 2020 09:56:17 -0000

On Tue, Aug 11, 2020 at 11:36 AM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Tue, Aug 11, 2020 at 4:38 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Tue, Aug 11, 2020 at 5:30 AM Hongtao Liu <crazylht@gmail.com> wrote:
> > >
> > > Hi:
> > >   The issue is described in the bugzilla.
> > >   Bootstrap is ok, regression test for i386/x86-64 backend is ok.
> > >   Ok for trunk?
> > >
> > > ChangeLog
> > > gcc/
> > >         PR target/96350
> > >         * config/i386/i386.c (ix86_legitimate_constant_p): Return
> > >         false for ENDBR immediate.
> > >         (ix86_legitimate_address_p): Ditto.
> > >         * config/i386/predicated.md
> > >         (x86_64_immediate_operand): Exclude ENDBR immediate.
> > >         (x86_64_zext_immediate_operand): Ditto.
> > >         (x86_64_dwzext_immediate_operand): Ditto.
> > >         (ix86_not_endbr_immediate_operand): New predicate.
> > >
> > > gcc/testsuite
> > >         * gcc.target/i386/endbr_immediate.c: New test.
> >
> > +;; Return true if VALUE isn't an ENDBR opcode in immediate field.
> > +(define_predicate "ix86_not_endbr_immediate_operand"
> > +  (match_test "1")
> >
> > Please reverse the above logic to introduce
> > ix86_endbr_immediate_operand, that returns true for unwanted
> > immediate. Something like:
> >
> > (define_predicate "ix86_endbr_immediate_operand"
> >   (match_code "const_int")
> > ...
> >
> > And you will be able to use it like:
> >
> > if (ix86_endbr_immediate_operand (x, VOIDmode)
> >   return false;
> >
>
> Changed.

No, it is not.

+  if ((flag_cf_protection & CF_BRANCH)
+      && CONST_INT_P (op))

You don't need to check for const ints here.

And please rewrite the body of the function to something like (untested):

{
  unsigned HOST_WIDE_INT val = TARGET_64BIT ? 0xfa1e0ff3 : 0xfb1e0ff3;

  if (x == val)
    return 1;

  if (TARGET_64BIT)
    for (; x >= val; x >>= 8)
      if (x == val)
        return 1;

  return 0;
}

so it will at least *look* like some thoughts have been spent on this.
I don't plan to review the code where it is obvious from the first
look that it was thrown together in a hurry. Please get some internal
company signoff first. Ping me in a week for a review.

Uros.
>
> >    /* Otherwise we handle everything else in the move patterns.  */
> > -  return true;
> > +  return ix86_not_endbr_immediate_operand (x, VOIDmode);
> >  }
> >
> > Please handle this in CASE_CONST_SCALAR_INT: part.
> >
> > +  if (disp && !ix86_not_endbr_immediate_operand (disp, VOIDmode))
> > +    return false;
> >
> > And this in:
> >
> >   /* Validate displacement.  */
> >   if (disp)
> >     {
> >
>
> Changed.

A better place for these new special cases is at the beginning of the
part I referred, not at the end.

Uros.