| Message ID | 20181220191653.8671-1-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [edk2] ArmPkg/ArmLib ARM: set .fpu to let Clang 7 assemble ArmV7Support.S | expand |
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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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? _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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
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(+) -- 2.19.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel