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.
Best regards,
Thomas
On Wed, 12 Dec 2018 at 10:30, Thomas Preudhomme
<thomas.preudho...@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.l...@linaro.org> wrote:
> >
> > On Tue, 11 Dec 2018 at 16:52, Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> > >
> > > Dimitar Dimitrov <dimi...@dinux.eu> writes:
> > > > On понеделник, 10 декември 2018 г. 11:21:53 EET Richard Sandiford wrote:
> > > >> Dimitar Dimitrov <dimi...@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 <dimi...@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 <dimi...@dinux.eu>
> > > >> >
> > > >> > * gcc.target/i386/pr52813.c: New test.
> > > >> >
> > > >> > Signed-off-by: Dimitar Dimitrov <dimi...@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
