Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <17600de3-b295-ebdf-b9fa-323d2bc3cb93@arm.com> <20220623174131.3818333-1-ndesaulniers@google.com>
 <6b330119-fbf2-1c34-7ad3-0ae789658d22@arm.com> <bd4a9bad394b478f9ee0d9c0981948f8@AcuMS.aculab.com>
In-Reply-To: <bd4a9bad394b478f9ee0d9c0981948f8@AcuMS.aculab.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Fri, 8 Jul 2022 16:04:10 -0700
Message-ID: <CAKwvOd=kcROA30fmviABhfUiDX2v8QYLPPmUV0hnQ2A29tMtDQ@mail.gmail.com>
Subject: Re: [PATCH v5] coresight: etm4x: avoid build failure with unrolled loops
To:     David Laight <David.Laight@aculab.com>
Cc:     Suzuki K Poulose <suzuki.poulose@arm.com>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Arnd Bergmann <arnd@kernel.org>,
        Tao Zhang <quic_taozha@quicinc.com>,
        Mike Leach <mike.leach@linaro.org>,
        Leo Yan <leo.yan@linaro.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Tom Rix <trix@redhat.com>,
        "coresight@lists.linaro.org" <coresight@lists.linaro.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "llvm@lists.linux.dev" <llvm@lists.linux.dev>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jun 28, 2022 at 2:40 AM David Laight <David.Laight@aculab.com> wrote:
>
> ...
> > > Regardless of which compiler or compiler options determine whether a
> > > loop can or can't be unrolled, which determines whether
> > > __builtin_constant_p evaluates to true when passed an expression using a
> > > loop induction variable, it is NEVER safe to allow the preprocessor to
> > > construct inline asm like:
> > >    asm volatile (".inst (0x160 + (i * 4))" : "=r"(__val));
> > >                                   ^ expected constant expression
>
> Can't you use (IIRC) an "=i" constraint with the C expression
> so that the compiler evaluates the expression and passes the
> correct constant value to the assembler?

Yes, though I think it may be even simpler for me to just use
__is_constexpr from include/linux/const.h here than try to rewrite the
existing macro soup to avoid calls to read_sysreg_s. Will send a
follow up.

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h
b/drivers/hwtracing/coresight/coresight-etm4x.h
index 33869c1d20c3..a7bfea31f7d8 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -7,6 +7,7 @@
 #define _CORESIGHT_CORESIGHT_ETM_H

 #include <asm/local.h>
+#include <linux/const.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include "coresight-priv.h"
@@ -515,7 +516,7 @@
        ({
         \
                u64 __val;
         \

         \
-               if (__builtin_constant_p((offset)))
         \
+               if (__is_constexpr((offset)))
         \
                        __val =
read_etm4x_sysreg_const_offset((offset));       \
                else
         \
                        __val = etm4x_sysreg_read((offset), true,
(_64bit));    \

--
Thanks,
~Nick Desaulniers