diff mbox series

[edk2,1/2] BaseTools/tools_def GCC: ARM/AARCH64: replace -save-temps with -pipe

Message ID 1497984234-19871-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit fa6080138c0804d4c094edfb8bfa37ab88935e45
Headers show
Series [edk2,1/2] BaseTools/tools_def GCC: ARM/AARCH64: replace -save-temps with -pipe | expand

Commit Message

Ard Biesheuvel June 20, 2017, 6:43 p.m. UTC 
For historical reasons, GCC builds for ARM and AARCH64 pass the
-save-temps command line option to GCC, which instructs the compiler
to preserve intermediate files, i.e., preprocessor output and generated
assembler. Given that this clutters up the Build directory, and slows
down the build, let's replace it with -pipe, which explicitly tells the
compiler to keep all intermediate representations in memory only.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 BaseTools/Conf/tools_def.template | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.7.4

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

Comments

Laszlo Ersek June 20, 2017, 7:28 p.m. UTC | #1 
On 06/20/17 20:43, Ard Biesheuvel wrote:
> For historical reasons, GCC builds for ARM and AARCH64 pass the

> -save-temps command line option to GCC, which instructs the compiler

> to preserve intermediate files, i.e., preprocessor output and generated

> assembler. Given that this clutters up the Build directory, and slows

> down the build, let's replace it with -pipe, which explicitly tells the

> compiler to keep all intermediate representations in memory only.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

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

>  1 file changed, 3 insertions(+), 3 deletions(-)

> 

> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

> index 04a1bcb210ab..7a58ce365ed2 100755

> --- a/BaseTools/Conf/tools_def.template

> +++ b/BaseTools/Conf/tools_def.template

> @@ -4399,7 +4399,7 @@ DEFINE GCC46_X64_DLINK_FLAGS         = DEF(GCC45_X64_DLINK_FLAGS)

>  DEFINE GCC46_X64_DLINK2_FLAGS        = DEF(GCC45_X64_DLINK2_FLAGS)

>  DEFINE GCC46_ASM_FLAGS               = DEF(GCC45_ASM_FLAGS)

>  DEFINE GCC46_ARM_ASM_FLAGS           = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian

> -DEFINE GCC46_ARM_CC_FLAGS            = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -save-temps

> +DEFINE GCC46_ARM_CC_FLAGS            = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -pipe

>  DEFINE GCC46_ARM_CC_XIPFLAGS         = -D__ARM_FEATURE_UNALIGNED=0

>  DEFINE GCC46_ARM_DLINK_FLAGS         = DEF(GCC_ARM_DLINK_FLAGS) -Wl,--oformat=elf32-littlearm

>  DEFINE GCC46_ARM_DLINK2_FLAGS        = DEF(GCC_DLINK2_FLAGS_COMMON) -Wl,--defsym=PECOFF_HEADER_SIZE=0x220

> @@ -4418,7 +4418,7 @@ DEFINE GCC47_ARM_ASM_FLAGS           = DEF(GCC46_ARM_ASM_FLAGS)

>  DEFINE GCC47_AARCH64_ASM_FLAGS       = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian

>  DEFINE GCC47_ARM_CC_FLAGS            = DEF(GCC46_ARM_CC_FLAGS)

>  DEFINE GCC47_ARM_CC_XIPFLAGS         = DEF(GCC_ARM_CC_XIPFLAGS)

> -DEFINE GCC47_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps

> +DEFINE GCC47_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -pipe

>  DEFINE GCC47_AARCH64_CC_XIPFLAGS     = DEF(GCC_AARCH64_CC_XIPFLAGS)

>  DEFINE GCC47_ARM_DLINK_FLAGS         = DEF(GCC46_ARM_DLINK_FLAGS)

>  DEFINE GCC47_ARM_DLINK2_FLAGS        = DEF(GCC46_ARM_DLINK2_FLAGS)

> @@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -save-temps

> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -pipe

>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

> 


"-pipe" isn't used anywhere else in "tools_def.template". Can we imagine
a platform where cross-compiling to aarch64 with gcc works otherwise,
but "-pipe" might break that? Cygwin perhaps? (No clue, honestly.)

For consistency with the rest of "tools_def.template", I'd suggest
simply dropping "-save-temps", and thinking about "-pipe" separately
(and then for all the GCC toolchains and for all arches). But, I don't
feel particularly strongly about this.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel June 21, 2017, 11:31 a.m. UTC | #2 
On 20 June 2017 at 21:28, Laszlo Ersek <lersek@redhat.com> wrote:
> On 06/20/17 20:43, Ard Biesheuvel wrote:

>> For historical reasons, GCC builds for ARM and AARCH64 pass the

>> -save-temps command line option to GCC, which instructs the compiler

>> to preserve intermediate files, i.e., preprocessor output and generated

>> assembler. Given that this clutters up the Build directory, and slows

>> down the build, let's replace it with -pipe, which explicitly tells the

>> compiler to keep all intermediate representations in memory only.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

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

>>  1 file changed, 3 insertions(+), 3 deletions(-)

>>

>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

>> index 04a1bcb210ab..7a58ce365ed2 100755

>> --- a/BaseTools/Conf/tools_def.template

>> +++ b/BaseTools/Conf/tools_def.template

>> @@ -4399,7 +4399,7 @@ DEFINE GCC46_X64_DLINK_FLAGS         = DEF(GCC45_X64_DLINK_FLAGS)

>>  DEFINE GCC46_X64_DLINK2_FLAGS        = DEF(GCC45_X64_DLINK2_FLAGS)

>>  DEFINE GCC46_ASM_FLAGS               = DEF(GCC45_ASM_FLAGS)

>>  DEFINE GCC46_ARM_ASM_FLAGS           = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian

>> -DEFINE GCC46_ARM_CC_FLAGS            = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -save-temps

>> +DEFINE GCC46_ARM_CC_FLAGS            = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -pipe

>>  DEFINE GCC46_ARM_CC_XIPFLAGS         = -D__ARM_FEATURE_UNALIGNED=0

>>  DEFINE GCC46_ARM_DLINK_FLAGS         = DEF(GCC_ARM_DLINK_FLAGS) -Wl,--oformat=elf32-littlearm

>>  DEFINE GCC46_ARM_DLINK2_FLAGS        = DEF(GCC_DLINK2_FLAGS_COMMON) -Wl,--defsym=PECOFF_HEADER_SIZE=0x220

>> @@ -4418,7 +4418,7 @@ DEFINE GCC47_ARM_ASM_FLAGS           = DEF(GCC46_ARM_ASM_FLAGS)

>>  DEFINE GCC47_AARCH64_ASM_FLAGS       = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian

>>  DEFINE GCC47_ARM_CC_FLAGS            = DEF(GCC46_ARM_CC_FLAGS)

>>  DEFINE GCC47_ARM_CC_XIPFLAGS         = DEF(GCC_ARM_CC_XIPFLAGS)

>> -DEFINE GCC47_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps

>> +DEFINE GCC47_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -pipe

>>  DEFINE GCC47_AARCH64_CC_XIPFLAGS     = DEF(GCC_AARCH64_CC_XIPFLAGS)

>>  DEFINE GCC47_ARM_DLINK_FLAGS         = DEF(GCC46_ARM_DLINK_FLAGS)

>>  DEFINE GCC47_ARM_DLINK2_FLAGS        = DEF(GCC46_ARM_DLINK2_FLAGS)

>> @@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

>>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

>>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

>>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

>> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -save-temps

>> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -pipe

>>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

>>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

>>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

>>

>

> "-pipe" isn't used anywhere else in "tools_def.template". Can we imagine

> a platform where cross-compiling to aarch64 with gcc works otherwise,

> but "-pipe" might break that? Cygwin perhaps? (No clue, honestly.)

>

> For consistency with the rest of "tools_def.template", I'd suggest

> simply dropping "-save-temps", and thinking about "-pipe" separately

> (and then for all the GCC toolchains and for all arches). But, I don't

> feel particularly strongly about this.

>


Yeah, that's a fair point. The fact that -save-temps is specific to
ARM does not mean we should perpetuate that with -pipe.

Leif, if you agree, I will drop the addition of -pipe from this patch,
and we can revisit it later for all GCC flavors.

-- 
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm June 21, 2017, 12:42 p.m. UTC | #3 
On Wed, Jun 21, 2017 at 01:31:15PM +0200, Ard Biesheuvel wrote:
> > "-pipe" isn't used anywhere else in "tools_def.template". Can we imagine

> > a platform where cross-compiling to aarch64 with gcc works otherwise,

> > but "-pipe" might break that? Cygwin perhaps? (No clue, honestly.)

> >

> > For consistency with the rest of "tools_def.template", I'd suggest

> > simply dropping "-save-temps", and thinking about "-pipe" separately

> > (and then for all the GCC toolchains and for all arches). But, I don't

> > feel particularly strongly about this.

> >

> 

> Yeah, that's a fair point. The fact that -save-temps is specific to

> ARM does not mean we should perpetuate that with -pipe.

> 

> Leif, if you agree, I will drop the addition of -pipe from this patch,

> and we can revisit it later for all GCC flavors.


Much as I would like to find out if there really are any platforms
that still have issues with -pipe (in which case, why wouldn't GCC
just use temp files anyway?), I guess that is a sensible approach.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

for the dropping of -save-temps.

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel June 22, 2017, 11:09 a.m. UTC | #4 
On 21 June 2017 at 12:42, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Jun 21, 2017 at 01:31:15PM +0200, Ard Biesheuvel wrote:

>> > "-pipe" isn't used anywhere else in "tools_def.template". Can we imagine

>> > a platform where cross-compiling to aarch64 with gcc works otherwise,

>> > but "-pipe" might break that? Cygwin perhaps? (No clue, honestly.)

>> >

>> > For consistency with the rest of "tools_def.template", I'd suggest

>> > simply dropping "-save-temps", and thinking about "-pipe" separately

>> > (and then for all the GCC toolchains and for all arches). But, I don't

>> > feel particularly strongly about this.

>> >

>>

>> Yeah, that's a fair point. The fact that -save-temps is specific to

>> ARM does not mean we should perpetuate that with -pipe.

>>

>> Leif, if you agree, I will drop the addition of -pipe from this patch,

>> and we can revisit it later for all GCC flavors.

>

> Much as I would like to find out if there really are any platforms

> that still have issues with -pipe (in which case, why wouldn't GCC

> just use temp files anyway?), I guess that is a sensible approach.

>

> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> for the dropping of -save-temps.

>


Thanks, pushed as fa6080138c08
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 04a1bcb210ab..7a58ce365ed2 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4399,7 +4399,7 @@  DEFINE GCC46_X64_DLINK_FLAGS         = DEF(GCC45_X64_DLINK_FLAGS)
 DEFINE GCC46_X64_DLINK2_FLAGS        = DEF(GCC45_X64_DLINK2_FLAGS)
 DEFINE GCC46_ASM_FLAGS               = DEF(GCC45_ASM_FLAGS)
 DEFINE GCC46_ARM_ASM_FLAGS           = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian
-DEFINE GCC46_ARM_CC_FLAGS            = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -save-temps
+DEFINE GCC46_ARM_CC_FLAGS            = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -pipe
 DEFINE GCC46_ARM_CC_XIPFLAGS         = -D__ARM_FEATURE_UNALIGNED=0
 DEFINE GCC46_ARM_DLINK_FLAGS         = DEF(GCC_ARM_DLINK_FLAGS) -Wl,--oformat=elf32-littlearm
 DEFINE GCC46_ARM_DLINK2_FLAGS        = DEF(GCC_DLINK2_FLAGS_COMMON) -Wl,--defsym=PECOFF_HEADER_SIZE=0x220
@@ -4418,7 +4418,7 @@  DEFINE GCC47_ARM_ASM_FLAGS           = DEF(GCC46_ARM_ASM_FLAGS)
 DEFINE GCC47_AARCH64_ASM_FLAGS       = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian
 DEFINE GCC47_ARM_CC_FLAGS            = DEF(GCC46_ARM_CC_FLAGS)
 DEFINE GCC47_ARM_CC_XIPFLAGS         = DEF(GCC_ARM_CC_XIPFLAGS)
-DEFINE GCC47_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps
+DEFINE GCC47_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -pipe
 DEFINE GCC47_AARCH64_CC_XIPFLAGS     = DEF(GCC_AARCH64_CC_XIPFLAGS)
 DEFINE GCC47_ARM_DLINK_FLAGS         = DEF(GCC46_ARM_DLINK_FLAGS)
 DEFINE GCC47_ARM_DLINK2_FLAGS        = DEF(GCC46_ARM_DLINK2_FLAGS)
@@ -4462,7 +4462,7 @@  DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)
 DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)
 DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)
 DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)
-DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -save-temps
+DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -pipe
 DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)
 DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)
 DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)