mbox series

[edk2,0/3] fixes for CLANG35 on ARM

Message ID 20181212103308.8099-1-ard.biesheuvel@linaro.org
Headers show
Series fixes for CLANG35 on ARM | expand

Message

Ard Biesheuvel Dec. 12, 2018, 10:33 a.m. UTC 
Building with Clang 3.5 for ARM may result in build breakage, due to the
fact that it may emit non-adjacent movw/movt instructions pairs which
cannot be relocated in PE/COFF. We pass -mno-movt in some places to
work around a related issue in the relocatable PrePi in ArmVirtPkg, but
we need to disable movw/movt entirely to really address this issue.

So first, fix some breakage that results from building with -mlong-calls
in the optimized BaseMemoryLib code (#1)

Patch #2 switches to -mkernel, which disables movw/movt generation (and
enabled -mlong-calls as a side effect)

Patch #3 removes the now redundant, and incompatible command line
overrides for the relocatable PrePi.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>

Ard Biesheuvel (3):
  MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations
  BaseTools/tools_def ARM CLANG35: work around -mno-movt option name
    change
  ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option

 ArmVirtPkg/ArmVirtQemuKernel.dsc                     | 5 -----
 ArmVirtPkg/ArmVirtXen.dsc                            | 5 -----
 BaseTools/Conf/tools_def.template                    | 2 +-
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S | 1 +
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S  | 1 +
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S     | 1 +
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S      | 5 +++++
 7 files changed, 9 insertions(+), 11 deletions(-)

-- 
2.19.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Dec. 12, 2018, 12:37 p.m. UTC | #1 
On Wed, Dec 12, 2018 at 11:33:05AM +0100, Ard Biesheuvel wrote:
> Building with Clang 3.5 for ARM may result in build breakage, due to the

> fact that it may emit non-adjacent movw/movt instructions pairs which

> cannot be relocated in PE/COFF. We pass -mno-movt in some places to

> work around a related issue in the relocatable PrePi in ArmVirtPkg, but

> we need to disable movw/movt entirely to really address this issue.

> 

> So first, fix some breakage that results from building with -mlong-calls

> in the optimized BaseMemoryLib code (#1)

> 

> Patch #2 switches to -mkernel, which disables movw/movt generation (and

> enabled -mlong-calls as a side effect)

> 

> Patch #3 removes the now redundant, and incompatible command line

> overrides for the relocatable PrePi.


For 1,3/3:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> Cc: Michael D Kinney <michael.d.kinney@intel.com>

> Cc: Liming Gao <liming.gao@intel.com>

> Cc: Bob Feng <bob.c.feng@intel.com>

> Cc: Leif Lindholm <leif.lindholm@linaro.org>

> Cc: Laszlo Ersek <lersek@redhat.com>

> 

> Ard Biesheuvel (3):

>   MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations

>   BaseTools/tools_def ARM CLANG35: work around -mno-movt option name

>     change

>   ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option

> 

>  ArmVirtPkg/ArmVirtQemuKernel.dsc                     | 5 -----

>  ArmVirtPkg/ArmVirtXen.dsc                            | 5 -----

>  BaseTools/Conf/tools_def.template                    | 2 +-

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S | 1 +

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S  | 1 +

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S     | 1 +

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S      | 5 +++++

>  7 files changed, 9 insertions(+), 11 deletions(-)

> 

> -- 

> 2.19.2

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Dec. 12, 2018, 2:01 p.m. UTC | #2 
Ard:
  I have no comments on this patch. So, CLANG38 has no issue. If so, could you recommend use CLANG38? 

Thanks
Liming
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Wednesday, December 12, 2018 6:33 PM

> To: edk2-devel@lists.01.org

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming

> <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Laszlo Ersek

> <lersek@redhat.com>

> Subject: [PATCH 0/3] fixes for CLANG35 on ARM

> 

> Building with Clang 3.5 for ARM may result in build breakage, due to the

> fact that it may emit non-adjacent movw/movt instructions pairs which

> cannot be relocated in PE/COFF. We pass -mno-movt in some places to

> work around a related issue in the relocatable PrePi in ArmVirtPkg, but

> we need to disable movw/movt entirely to really address this issue.

> 

> So first, fix some breakage that results from building with -mlong-calls

> in the optimized BaseMemoryLib code (#1)

> 

> Patch #2 switches to -mkernel, which disables movw/movt generation (and

> enabled -mlong-calls as a side effect)

> 

> Patch #3 removes the now redundant, and incompatible command line

> overrides for the relocatable PrePi.

> 

> Cc: Michael D Kinney <michael.d.kinney@intel.com>

> Cc: Liming Gao <liming.gao@intel.com>

> Cc: Bob Feng <bob.c.feng@intel.com>

> Cc: Leif Lindholm <leif.lindholm@linaro.org>

> Cc: Laszlo Ersek <lersek@redhat.com>

> 

> Ard Biesheuvel (3):

>   MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations

>   BaseTools/tools_def ARM CLANG35: work around -mno-movt option name

>     change

>   ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option

> 

>  ArmVirtPkg/ArmVirtQemuKernel.dsc                     | 5 -----

>  ArmVirtPkg/ArmVirtXen.dsc                            | 5 -----

>  BaseTools/Conf/tools_def.template                    | 2 +-

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S | 1 +

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S  | 1 +

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S     | 1 +

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S      | 5 +++++

>  7 files changed, 9 insertions(+), 11 deletions(-)

> 

> --

> 2.19.2


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 12, 2018, 2:02 p.m. UTC | #3 
On Wed, 12 Dec 2018 at 15:01, Gao, Liming <liming.gao@intel.com> wrote:
>

> Ard:

>   I have no comments on this patch. So, CLANG38 has no issue. If so, could you recommend use CLANG38?

>


Yes, the latest is always preferred. However, since CLANG38 enables
LTO, you need the LLVMgold plugin, which is not shipped for all
versions of Clang by the distros. So it is good to keep CLANG35 as a
fallback.

> > -----Original Message-----

> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > Sent: Wednesday, December 12, 2018 6:33 PM

> > To: edk2-devel@lists.01.org

> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming

> > <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Laszlo Ersek

> > <lersek@redhat.com>

> > Subject: [PATCH 0/3] fixes for CLANG35 on ARM

> >

> > Building with Clang 3.5 for ARM may result in build breakage, due to the

> > fact that it may emit non-adjacent movw/movt instructions pairs which

> > cannot be relocated in PE/COFF. We pass -mno-movt in some places to

> > work around a related issue in the relocatable PrePi in ArmVirtPkg, but

> > we need to disable movw/movt entirely to really address this issue.

> >

> > So first, fix some breakage that results from building with -mlong-calls

> > in the optimized BaseMemoryLib code (#1)

> >

> > Patch #2 switches to -mkernel, which disables movw/movt generation (and

> > enabled -mlong-calls as a side effect)

> >

> > Patch #3 removes the now redundant, and incompatible command line

> > overrides for the relocatable PrePi.

> >

> > Cc: Michael D Kinney <michael.d.kinney@intel.com>

> > Cc: Liming Gao <liming.gao@intel.com>

> > Cc: Bob Feng <bob.c.feng@intel.com>

> > Cc: Leif Lindholm <leif.lindholm@linaro.org>

> > Cc: Laszlo Ersek <lersek@redhat.com>

> >

> > Ard Biesheuvel (3):

> >   MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations

> >   BaseTools/tools_def ARM CLANG35: work around -mno-movt option name

> >     change

> >   ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option

> >

> >  ArmVirtPkg/ArmVirtQemuKernel.dsc                     | 5 -----

> >  ArmVirtPkg/ArmVirtXen.dsc                            | 5 -----

> >  BaseTools/Conf/tools_def.template                    | 2 +-

> >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S | 1 +

> >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S  | 1 +

> >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S     | 1 +

> >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S      | 5 +++++

> >  7 files changed, 9 insertions(+), 11 deletions(-)

> >

> > --

> > 2.19.2

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Dec. 12, 2018, 2:19 p.m. UTC | #4 
Make sense. So, CLANG35 tool chain can also be used by CLANG 3.5 and above version compiler, like GCC49?

> -----Original Message-----

> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel

> Sent: Wednesday, December 12, 2018 10:03 PM

> To: Gao, Liming <liming.gao@intel.com>

> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>

> Subject: Re: [edk2] [PATCH 0/3] fixes for CLANG35 on ARM

> 

> On Wed, 12 Dec 2018 at 15:01, Gao, Liming <liming.gao@intel.com> wrote:

> >

> > Ard:

> >   I have no comments on this patch. So, CLANG38 has no issue. If so, could you recommend use CLANG38?

> >

> 

> Yes, the latest is always preferred. However, since CLANG38 enables

> LTO, you need the LLVMgold plugin, which is not shipped for all

> versions of Clang by the distros. So it is good to keep CLANG35 as a

> fallback.

> 

> > > -----Original Message-----

> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > Sent: Wednesday, December 12, 2018 6:33 PM

> > > To: edk2-devel@lists.01.org

> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming

> > > <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Laszlo Ersek

> > > <lersek@redhat.com>

> > > Subject: [PATCH 0/3] fixes for CLANG35 on ARM

> > >

> > > Building with Clang 3.5 for ARM may result in build breakage, due to the

> > > fact that it may emit non-adjacent movw/movt instructions pairs which

> > > cannot be relocated in PE/COFF. We pass -mno-movt in some places to

> > > work around a related issue in the relocatable PrePi in ArmVirtPkg, but

> > > we need to disable movw/movt entirely to really address this issue.

> > >

> > > So first, fix some breakage that results from building with -mlong-calls

> > > in the optimized BaseMemoryLib code (#1)

> > >

> > > Patch #2 switches to -mkernel, which disables movw/movt generation (and

> > > enabled -mlong-calls as a side effect)

> > >

> > > Patch #3 removes the now redundant, and incompatible command line

> > > overrides for the relocatable PrePi.

> > >

> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>

> > > Cc: Liming Gao <liming.gao@intel.com>

> > > Cc: Bob Feng <bob.c.feng@intel.com>

> > > Cc: Leif Lindholm <leif.lindholm@linaro.org>

> > > Cc: Laszlo Ersek <lersek@redhat.com>

> > >

> > > Ard Biesheuvel (3):

> > >   MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations

> > >   BaseTools/tools_def ARM CLANG35: work around -mno-movt option name

> > >     change

> > >   ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option

> > >

> > >  ArmVirtPkg/ArmVirtQemuKernel.dsc                     | 5 -----

> > >  ArmVirtPkg/ArmVirtXen.dsc                            | 5 -----

> > >  BaseTools/Conf/tools_def.template                    | 2 +-

> > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S | 1 +

> > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S  | 1 +

> > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S     | 1 +

> > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S      | 5 +++++

> > >  7 files changed, 9 insertions(+), 11 deletions(-)

> > >

> > > --

> > > 2.19.2

> >

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 12, 2018, 2:19 p.m. UTC | #5 
On Wed, 12 Dec 2018 at 15:19, Gao, Liming <liming.gao@intel.com> wrote:
>

> Make sense. So, CLANG35 tool chain can also be used by CLANG 3.5 and above version compiler, like GCC49?

>


Exactly

> > -----Original Message-----

> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel

> > Sent: Wednesday, December 12, 2018 10:03 PM

> > To: Gao, Liming <liming.gao@intel.com>

> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>

> > Subject: Re: [edk2] [PATCH 0/3] fixes for CLANG35 on ARM

> >

> > On Wed, 12 Dec 2018 at 15:01, Gao, Liming <liming.gao@intel.com> wrote:

> > >

> > > Ard:

> > >   I have no comments on this patch. So, CLANG38 has no issue. If so, could you recommend use CLANG38?

> > >

> >

> > Yes, the latest is always preferred. However, since CLANG38 enables

> > LTO, you need the LLVMgold plugin, which is not shipped for all

> > versions of Clang by the distros. So it is good to keep CLANG35 as a

> > fallback.

> >

> > > > -----Original Message-----

> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > > > Sent: Wednesday, December 12, 2018 6:33 PM

> > > > To: edk2-devel@lists.01.org

> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming

> > > > <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Laszlo Ersek

> > > > <lersek@redhat.com>

> > > > Subject: [PATCH 0/3] fixes for CLANG35 on ARM

> > > >

> > > > Building with Clang 3.5 for ARM may result in build breakage, due to the

> > > > fact that it may emit non-adjacent movw/movt instructions pairs which

> > > > cannot be relocated in PE/COFF. We pass -mno-movt in some places to

> > > > work around a related issue in the relocatable PrePi in ArmVirtPkg, but

> > > > we need to disable movw/movt entirely to really address this issue.

> > > >

> > > > So first, fix some breakage that results from building with -mlong-calls

> > > > in the optimized BaseMemoryLib code (#1)

> > > >

> > > > Patch #2 switches to -mkernel, which disables movw/movt generation (and

> > > > enabled -mlong-calls as a side effect)

> > > >

> > > > Patch #3 removes the now redundant, and incompatible command line

> > > > overrides for the relocatable PrePi.

> > > >

> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>

> > > > Cc: Liming Gao <liming.gao@intel.com>

> > > > Cc: Bob Feng <bob.c.feng@intel.com>

> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org>

> > > > Cc: Laszlo Ersek <lersek@redhat.com>

> > > >

> > > > Ard Biesheuvel (3):

> > > >   MdePkg/BaseMemoryLibOptDxe ARM: add missing function annotations

> > > >   BaseTools/tools_def ARM CLANG35: work around -mno-movt option name

> > > >     change

> > > >   ArmVirtPkg/PrePi ARM CLANG35: drop incompatible command line option

> > > >

> > > >  ArmVirtPkg/ArmVirtQemuKernel.dsc                     | 5 -----

> > > >  ArmVirtPkg/ArmVirtXen.dsc                            | 5 -----

> > > >  BaseTools/Conf/tools_def.template                    | 2 +-

> > > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S | 1 +

> > > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S  | 1 +

> > > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S     | 1 +

> > > >  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S      | 5 +++++

> > > >  7 files changed, 9 insertions(+), 11 deletions(-)

> > > >

> > > > --

> > > > 2.19.2

> > >

> > _______________________________________________

> > edk2-devel mailing list

> > edk2-devel@lists.01.org

> > https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 13, 2018, 10:49 a.m. UTC | #6 
On Wed, 12 Dec 2018 at 15:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>

> On Wed, 12 Dec 2018 at 15:19, Gao, Liming <liming.gao@intel.com> wrote:

> >

> > Make sense. So, CLANG35 tool chain can also be used by CLANG 3.5 and above version compiler, like GCC49?

> >

>

> Exactly

>


Liming,

Are you happy with the MdePkg and BaseTools changes in this series?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Dec. 13, 2018, 11:42 a.m. UTC | #7 
Yes. Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Thursday, December 13, 2018 6:49 PM

> To: Gao, Liming <liming.gao@intel.com>

> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>

> Subject: Re: [edk2] [PATCH 0/3] fixes for CLANG35 on ARM

> 

> On Wed, 12 Dec 2018 at 15:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> >

> > On Wed, 12 Dec 2018 at 15:19, Gao, Liming <liming.gao@intel.com> wrote:

> > >

> > > Make sense. So, CLANG35 tool chain can also be used by CLANG 3.5 and above version compiler, like GCC49?

> > >

> >

> > Exactly

> >

> 

> Liming,

> 

> Are you happy with the MdePkg and BaseTools changes in this series?

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 13, 2018, 11:49 a.m. UTC | #8 
On Thu, 13 Dec 2018 at 12:42, Gao, Liming <liming.gao@intel.com> wrote:
>

> Yes. Reviewed-by: Liming Gao <liming.gao@intel.com>

>


Thanks all

Pushed as 580f4539dfbb..36deafb838d0

> > -----Original Message-----

> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> > Sent: Thursday, December 13, 2018 6:49 PM

> > To: Gao, Liming <liming.gao@intel.com>

> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>

> > Subject: Re: [edk2] [PATCH 0/3] fixes for CLANG35 on ARM

> >

> > On Wed, 12 Dec 2018 at 15:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> > >

> > > On Wed, 12 Dec 2018 at 15:19, Gao, Liming <liming.gao@intel.com> wrote:

> > > >

> > > > Make sense. So, CLANG35 tool chain can also be used by CLANG 3.5 and above version compiler, like GCC49?

> > > >

> > >

> > > Exactly

> > >

> >

> > Liming,

> >

> > Are you happy with the MdePkg and BaseTools changes in this series?

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel