This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] [RFC] PR target/52813 and target/11807
[resending from the right address]
Hi Christophe,
Why not simply: "Clobber of <REGISTER> unsupported" with an
accompanying change of the documentation to state the extra bit you
wanted to put in that error message? Perhaps even add a reference to
the section of the documentation in the error message.
Best regards,
Thomas
On Wed, 12 Dec 2018 at 15:13, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
>
> On Wed, 12 Dec 2018 at 14:19, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
> >
> > On Wed, 12 Dec 2018 at 12:21, Thomas Preudhomme
> > <thomas.preudhomme@linaro.org> wrote:
> > >
> > > So my understanding is that the original code (CMSIS library) used to
> > > clobber sp because the asm statement was actually changing the sp.
> > > That in turn led GCC to try to save and restore sp which is not what
> > > CMSIS was expecting to happen. Changing sp without clobber as done now
> > > is probably the right solution and r242693 can be reverted. That will
> > > remove the failing test.
> > >
> >
> > OK, I read PR52813 too, but I'm not sure to fully understand the new status.
> > My understanding is that since this patch was committed, if an asm statement
> > clobbers sp, it is now allowed to actually declare it as clobber (this patch
> > generates an error in such a case).
> > So the user is now expected to lie to the compiler when writing to
> > this kind of register (sp, pic register), by not declaring it as "clobber"?
> >
>
> I'm attaching a small patch which adds a more verbose error message
> along the lines of what I understand of the current status.
> I'm pretty sure I got (at least) the formatting wrong :)
>
> Christophe
>
> >
> > > Best regards,
> > >
> > > Thomas
> > > On Wed, 12 Dec 2018 at 10:30, Thomas Preudhomme
> > > <thomas.preudhomme@linaro.org> wrote:
> > > >
> > > > Hi Christophe,
> > > >
> > > > That PR was about a bug occuring when sp was clobbered so if it cannot
> > > > be clobbered anymore the whole commit (r242693) can be removed. Let me
> > > > check the original code that lead to the PR why it's clobbering sp
> > > > though.
> > > >
> > > > Best regards,
> > > >
> > > > Thomas
> > > > On Wed, 12 Dec 2018 at 09:43, Christophe Lyon
> > > > <christophe.lyon@linaro.org> wrote:
> > > > >
> > > > > On Tue, 11 Dec 2018 at 16:52, Richard Sandiford
> > > > > <richard.sandiford@arm.com> wrote:
> > > > > >
> > > > > > Dimitar Dimitrov <dimitar@dinux.eu> writes:
> > > > > > > On понеделник, 10 декември 2018 г. 11:21:53 EET Richard Sandiford wrote:
> > > > > > >> Dimitar Dimitrov <dimitar@dinux.eu> writes:
> > > > > > >> > I have tested this fix on x86_64 host, and found no regression in the C
> > > > > > >> > and C++ testsuites. I'm marking this patch as RFC simply because I don't
> > > > > > >> > have experience with other architectures, and I don't have a setup to
> > > > > > >> > test all architectures supported by GCC.
> > > > > > >> >
> > > > > > >> > gcc/ChangeLog:
> > > > > > >> >
> > > > > > >> > 2018-12-07 Dimitar Dimitrov <dimitar@dinux.eu>
> > > > > > >> >
> > > > > > >> > * cfgexpand.c (asm_clobber_reg_is_valid): Also produce
> > > > > > >> > error when stack pointer is clobbered.
> > > > > > >> > (expand_asm_stmt): Refactor clobber check in separate function.
> > > > > > >> >
> > > > > > >> > gcc/testsuite/ChangeLog:
> > > > > > >> >
> > > > > > >> > 2018-12-07 Dimitar Dimitrov <dimitar@dinux.eu>
> > > > > > >> >
> > > > > > >> > * gcc.target/i386/pr52813.c: New test.
> > > > > > >> >
> > > > > > >> > Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
> > > > > > >>
> > > > > > >> LGTM. Do you have a copyright assignment on file? 'Fraid this is
> > > > > > >> probably big enough to need one.
> > > > > > > Yes, I have copyright assignment.
> > > > > >
> > > > > > OK, great. I went ahead and applied the patch.
> > > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > This patch introduces a regression on arm:
> > > > > FAIL: gcc.target/arm/pr77904.c (test for excess errors)
> > > > > Excess errors:
> > > > > /gcc/testsuite/gcc.target/arm/pr77904.c:7:3: error: Stack Pointer
> > > > > register clobbered by 'sp' in 'asm'
> > > > >
> > > > > Indeed the testcase has an explicit:
> > > > > __asm volatile ("" : : : "sp");
> > > > > which is now rejected.
> > > > >
> > > > > Thomas, is that mandatory to test your code to fix pr77904?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Christophe
> > > > >
> > > > > > Thanks,
> > > > > > Richard