* [PATCH] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S @ 2018-12-20 19:16 Ard Biesheuvel 2018-12-21 17:58 ` Leif Lindholm 0 siblings, 1 reply; 4+ messages in thread From: Ard Biesheuvel @ 2018-12-20 19:16 UTC (permalink / raw) To: edk2-devel Clang 7 complains about the vmsr instruction in ArmV7Support.S, which is only available on cores that implement some flavour of VFP. So set the .fpu to NEON like we do in some other places. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 1 + 1 file changed, 1 insertion(+) diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S index 281499b46cbc..1808962ee3e2 100644 --- a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S +++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S @@ -268,6 +268,7 @@ ASM_FUNC(ArmEnableVFP) #ifndef __clang__ mcr p10,#0x7,r0,c8,c0,#0 #else + .fpu neon vmsr fpexc, r0 #endif bx lr -- 2.19.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S 2018-12-20 19:16 [PATCH] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S Ard Biesheuvel @ 2018-12-21 17:58 ` Leif Lindholm 2018-12-21 18:00 ` Ard Biesheuvel 0 siblings, 1 reply; 4+ messages in thread From: Leif Lindholm @ 2018-12-21 17:58 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel On Thu, Dec 20, 2018 at 08:16:53PM +0100, Ard Biesheuvel wrote: > Clang 7 complains about the vmsr instruction in ArmV7Support.S, > which is only available on cores that implement some flavour of > VFP. So set the .fpu to NEON like we do in some other places. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > index 281499b46cbc..1808962ee3e2 100644 > --- a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > +++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > @@ -268,6 +268,7 @@ ASM_FUNC(ArmEnableVFP) > #ifndef __clang__ > mcr p10,#0x7,r0,c8,c0,#0 > #else > + .fpu neon > vmsr fpexc, r0 > #endif No objection from me. But I would point out that the special clang filtering here could possibly be dropped. I mean, theoretically someone could have an even older binutils, but Linaro GCC 4.8-2013.05 will happily assemble the clang side of this conditional. / Leif ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S 2018-12-21 17:58 ` Leif Lindholm @ 2018-12-21 18:00 ` Ard Biesheuvel 2018-12-21 19:50 ` Leif Lindholm 0 siblings, 1 reply; 4+ messages in thread From: Ard Biesheuvel @ 2018-12-21 18:00 UTC (permalink / raw) To: Leif Lindholm; +Cc: edk2-devel@lists.01.org On Fri, 21 Dec 2018 at 18:58, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Thu, Dec 20, 2018 at 08:16:53PM +0100, Ard Biesheuvel wrote: > > Clang 7 complains about the vmsr instruction in ArmV7Support.S, > > which is only available on cores that implement some flavour of > > VFP. So set the .fpu to NEON like we do in some other places. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > > index 281499b46cbc..1808962ee3e2 100644 > > --- a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > > +++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > > @@ -268,6 +268,7 @@ ASM_FUNC(ArmEnableVFP) > > #ifndef __clang__ > > mcr p10,#0x7,r0,c8,c0,#0 > > #else > > + .fpu neon > > vmsr fpexc, r0 > > #endif > > No objection from me. But I would point out that the special clang > filtering here could possibly be dropped. > > I mean, theoretically someone could have an even older binutils, but > Linaro GCC 4.8-2013.05 will happily assemble the clang side of this > conditional. > Yeah, but that would be a separate patch, no? ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S 2018-12-21 18:00 ` Ard Biesheuvel @ 2018-12-21 19:50 ` Leif Lindholm 0 siblings, 0 replies; 4+ messages in thread From: Leif Lindholm @ 2018-12-21 19:50 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Andrew Fish On Fri, Dec 21, 2018 at 07:00:04PM +0100, Ard Biesheuvel wrote: > On Fri, 21 Dec 2018 at 18:58, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > > > On Thu, Dec 20, 2018 at 08:16:53PM +0100, Ard Biesheuvel wrote: > > > Clang 7 complains about the vmsr instruction in ArmV7Support.S, > > > which is only available on cores that implement some flavour of > > > VFP. So set the .fpu to NEON like we do in some other places. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > --- > > > ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > > > index 281499b46cbc..1808962ee3e2 100644 > > > --- a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > > > +++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.S > > > @@ -268,6 +268,7 @@ ASM_FUNC(ArmEnableVFP) > > > #ifndef __clang__ > > > mcr p10,#0x7,r0,c8,c0,#0 > > > #else > > > + .fpu neon > > > vmsr fpexc, r0 > > > #endif > > > > No objection from me. But I would point out that the special clang > > filtering here could possibly be dropped. > > > > I mean, theoretically someone could have an even older binutils, but > > Linaro GCC 4.8-2013.05 will happily assemble the clang side of this > > conditional. > > Yeah, but that would be a separate patch, no? Oh, sure. Just, it would be nice if that could precede this change. That said, the other two .fpu neon instances in edk2 are both hidden behind #if !defined(__APPLE__). Should we check with Andrew (post-wilderbeest) if this is something we need to keep (and if so add it here) or whether we can delete it across the board? / Leif ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-21 19:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-20 19:16 [PATCH] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S Ard Biesheuvel 2018-12-21 17:58 ` Leif Lindholm 2018-12-21 18:00 ` Ard Biesheuvel 2018-12-21 19:50 ` Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox