P8 build fixes for modern GCC #193

shenki · 2020-04-16T06:06:46Z

No description provided.

ifcompiler/initCompiler.C: In constructor ‘init::Parser::Parser(int, char**)’: ifcompiler/initCompiler.C:311:18: error: no match for ‘operator<<’ (operand types are ‘std::ostream’ {aka ‘std::basic_ostream<char>’} and ‘std::ostringstream’ {aka ‘std::__cxx11::basic_ostringstream<char>’}) 311 | cout << stats; | ~~~~ ^~ ~~~~~ | | | | | std::ostringstream {aka std::__cxx11::basic_ostringstream<char>} | std::ostream {aka std::basic_ostream<char>} Change-Id: I563b5008050b2ea2baa456226cc2afbdbd63805d Signed-off-by: Joel Stanley <[email protected]>

Resolve warnings when compiling with gcc 4.8. Compiled with GCC 7.3, no more compile errors/warnings; build ends with caught exception from linker. This commit compiles with GCC 8.2, no more error/warnings; except for a linking warning. Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/72271 (cherry picked from p9 branch) Signed-off-by: Joel Stanley <[email protected]> Change-Id: Iff33e15367cea0b8693709941592a2275efac589

Change-Id: I52e002a2f7085baa5b6b5d536e5aa95a5fab2c0d RTC: 123492 Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/26507 Tested-by: Jenkins Server <[email protected]> Tested-by: FSP CI Jenkins <[email protected]> Reviewed-by: Christian R. Geddes <[email protected]> Reviewed-by: William G. Hoffa <[email protected]> Reviewed-by: Matthew A. Ploetz <[email protected]> Signed-off-by: Joel Stanley <[email protected]>

Change-Id: Id8c8be0ef1d5e9f3ca377c4a89715c9c46d17a38 Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/38304 Reviewed-by: Prachi Gupta <[email protected]> Tested-by: Jenkins Server <[email protected]> Reviewed-by: Elizabeth K. Liner <[email protected]> Tested-by: FSP CI Jenkins <[email protected]> Tested-by: Jenkins OP Build CI <[email protected]> Reviewed-by: Matthew A. Ploetz <[email protected]> Signed-off-by: Joel Stanley <[email protected]>

Change-Id: I381732be6facd25fb27bbd1f8ac0d75eb5b4980e

GCC4.9 defaults to ELFv2 ABI support but we do not have all of the needed tools to suppor this. Change-Id: Iacf49c46b1fb25776ba60d6506ccadff7b46bf60 RTC: 123430

Seeing as no C++ dialect was previously selected, GCC < 6 defaulted to GNU++98 as the standard C++ mode. However... it seems that C++03 is a better match for HostBoot code (confirmed by Patrick Williams in open-power#62 (comment) ) Change-Id: I6661b5b61319c85c2a90926beb6e2662e96dcf06 Signed-off-by: Stewart Smith <[email protected]>

… 'void' Change-Id: I9dc8b698fec95488e209cbc40b928769a26b6de6 Signed-off-by: Stewart Smith <[email protected]>

…ules Change-Id: I63accd3e881c941736ece4b4498c2c9d06ff8761 Signed-off-by: Stewart Smith <[email protected]>

GCC6 throws the following error: operand type ‘bool*’ is incompatible with argument 1 of ‘__sync_fetch_and_and’ GCC documents that bool is invalid for __sync builtins over at: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins " GCC allows any scalar type that is 1, 2, 4 or 8 bytes in size other than the C type _Bool or the C++ type bool" Change-Id: Iab6415348cb19f590921d8ccc5349867fa57a42d Signed-off-by: Stewart Smith <[email protected]>

…NULL PRDF_ASSERT( &r != NULL ); Change-Id: I2d60075f9e2232512efe45a5c6aa5563f3a565f5 Signed-off-by: Stewart Smith <[email protected]>

So, it turns out that relying on the address of something passed by reference being able to be NULL isn't exactly a good idea, or remotely obvious code. So, instead, do the sane thing and pass a pointer and check it. ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H: In constructor ‘PRDF::AttnTypeRegister::AttnTypeRegister(PRDF::SCAN_COMM_REGISTE R_CLASS&, PRDF::SCAN_COMM_REGISTER_CLASS&, PRDF::SCAN_COMM_REGISTER_CLASS&, PRDF ::SCAN_COMM_REGISTER_CLASS&)’: ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 42:21: error: the compiler can assume that the address of ‘i_check’ will never b e NULL [-Werror=address] iv_check( NULL == &i_check ? &cv_null : &i_check), ~~^~~~~~~~~~~ ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 43:21: error: the compiler can assume that the address of ‘i_recov’ will never b e NULL [-Werror=address] iv_recov( NULL == &i_recov ? &cv_null : &i_recov), ~~^~~~~~~~~~~ ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 44:22: error: the compiler can assume that the address of ‘i_special’ will never be NULL [-Werror=address] iv_special(NULL == &i_special ? &cv_null : &i_special), ~~^~~~~~~~~~~~~ ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 45:22: error: the compiler can assume that the address of ‘i_proccs’ will never be NULL [-Werror=address] iv_proccs( NULL == &i_proccs ? &cv_null : &i_proccs), ~~^~~~~~~~~~~~ Change-Id: Iecd8636da67aac24f64f73fd82b1f7ccbfced900 Signed-off-by: Stewart Smith <[email protected]>

fix GCC6 thrown error Change-Id: I9aa508843f54c99ebb59822c39590811423ad0b1 Signed-off-by: Stewart Smith <[email protected]>

Seeing as the ancient GCC on RHEL6 doesn't actually support -std=gnu++03 we have to go through some hoops to detect it (we use the same magic Make as we use in skiboot to do the same) Change-Id: I338560ae2ebdac842c8055c07519d542564c3923 Signed-off-by: Stewart Smith <[email protected]>

From Linux 80f23935cadb ("powerpc: Convert cmp to cmpd in idle enter sequence"): PowerPC's "cmp" instruction has four operands. Normally people write "cmpw" or "cmpd" for the second cmp operand 0 or 1. But, frequently people forget, and write "cmp" with just three operands. With older binutils this is silently accepted as if this was "cmpw", while often "cmpd" is wanted. With newer binutils GAS will complain about this for 64-bit code. For 32-bit code it still silently assumes "cmpw" is what is meant. The first two instances are dealing with SPRs, which are 32-bit, so cmpw is correct. It is not clear what to do in the third use of cmp, but given old toolchains have generated cmpw lets assume that we should maintain the behaviour. Change-Id: Iee5dd5903dcd7ac4028bab0176e08ce3db23b2d5 Signed-off-by: Joel Stanley <[email protected]>

dcrowell77 · 2020-04-22T21:44:55Z

FYI - Not ignoring this, but some of these changes are incompatible with the GCC we're using internally. Trying to figure out how to conditionally include some of the GCC flags, e.g. -mabi=elfv1

shenki · 2020-04-22T23:05:00Z

Would this work?

From 33947245ccc71d19b8ec1ffb8be1880459521da6 Mon Sep 17 00:00:00 2001
From: Joel Stanley <[email protected]>
Date: Thu, 23 Apr 2020 08:32:01 +0930
Subject: [PATCH] cflags: Avoid -mabi=elfv1 for old tools

IBM has some old toolchains that need to build this code base that do
not support the elfv1 flag.

As the cflags.env.mk does not know about the compiler settings, put them
in env.mk after all the other makefiles have been included.

Change-Id: I13e002f31f481a14a4a1295420f4fd7f295f74e0
Signed-off-by: Joel Stanley <[email protected]>
---
 src/build/mkrules/cflags.env.mk | 7 +++----
 src/build/mkrules/env.mk        | 8 +++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/build/mkrules/cflags.env.mk b/src/build/mkrules/cflags.env.mk
index 971c11b9ad89..ccd71e6d1086 100644
--- a/src/build/mkrules/cflags.env.mk
+++ b/src/build/mkrules/cflags.env.mk
@@ -5,7 +5,7 @@
 #
 # OpenPOWER HostBoot Project
 #
-# Contributors Listed Below - COPYRIGHT 2013,2015
+# Contributors Listed Below - COPYRIGHT 2013,2020
 # [+] Google Inc.
 # [+] International Business Machines Corp.
 #
@@ -39,9 +39,8 @@ COMMONFLAGS += $(OPT_LEVEL) -nostdlib
 # TODO RTC: 123994 - ELFv2 ABI support (-mabi=elfv1)
 CFLAGS += $(COMMONFLAGS) -mcpu=power7 -nostdinc -g -mno-vsx -mno-altivec\
           -Wall -Werror -mtraceback=no -pipe \
-	  -ffunction-sections -fdata-sections -ffreestanding -mbig-endian \
-      -mabi=elfv1
-ASMFLAGS += $(COMMONFLAGS) -mcpu=power7 -mbig-endian -mabi=elfv1
+	  -ffunction-sections -fdata-sections -ffreestanding -mbig-endian
+ASMFLAGS += $(COMMONFLAGS) -mcpu=power7 -mbig-endian
 CXXFLAGS += $(CFLAGS) -nostdinc++ -fno-rtti -fno-exceptions -Wall \
 	    -fuse-cxa-atexit -std=gnu++03
 LDFLAGS += --nostdlib --sort-common -EB $(COMMONFLAGS)
diff --git a/src/build/mkrules/env.mk b/src/build/mkrules/env.mk
index 4eacbaa3d607..dc0ec171f139 100644
--- a/src/build/mkrules/env.mk
+++ b/src/build/mkrules/env.mk
@@ -5,7 +5,9 @@
 #
 # OpenPOWER HostBoot Project
 #
-# COPYRIGHT International Business Machines Corp. 2013,2014
+# Contributors Listed Below - COPYRIGHT 2013,2020
+# [+] International Business Machines Corp.
+#
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -78,3 +80,7 @@ include $(MKRULESDIR)/binfile.env.mk
 include $(MKRULESDIR)/beam.env.mk
 include $(MKRULESDIR)/gcov.env.mk
 include $(MKRULESDIR)/cflags.env.mk
+
+ELFV1 := $(call try-cflags,$(CC),-mabi=elfv1)
+CFLAGS += $(ELFV1)
+COMMONFLAGS += $(ELFV1)
-- 
2.26.1

klauskiwi · 2020-05-14T20:04:53Z

Any new news here @dcrowell77 ? Have these been integrated?

dcrowell77 · 2020-05-15T03:10:31Z

No news, haven't had time to dig in.

paulmenzel · 2020-06-17T05:20:23Z

src/usr/console/uart.C

@@ -117,7 +117,7 @@ namespace CONSOLE
            // Wait for transmit FIFO to have space.
            {
                const uint64_t DELAY_NS = 100;
-                const uint64_t DELAY_LOOPS = 10000;
+                const uint64_t DELAY_LOOPS = 100000000;


Could you add an explanation for this in the commit message please?

I've got no idea why this was done. The commit message from 2014 was as follows:

Changes for Habanero bringup, uart delay and centaur/vddr

Note that every openpower machine has carried this change. It was placed in the op-build tree instead of committed to the hostboot repository for unknown reasons.

I have no explanation beyond wishful thinking on why it remained as a patch for so long.

shenki and others added 15 commits January 3, 2020 18:25

Changes for Habanero bringup, uart delay and centaur/vddr

be52281

Change-Id: I381732be6facd25fb27bbd1f8ac0d75eb5b4980e

GCC 4.9: Make compiler use ELFv1 ABI.

8abcd70

GCC4.9 defaults to ELFv2 ABI support but we do not have all of the needed tools to suppor this. Change-Id: Iacf49c46b1fb25776ba60d6506ccadff7b46bf60 RTC: 123430

fix build error: return-statement with a value, in function returning…

5e6d9e4

… 'void' Change-Id: I9dc8b698fec95488e209cbc40b928769a26b6de6 Signed-off-by: Stewart Smith <[email protected]>

error: dereferencing type-punned pointer will break strict-aliasing r…

e1cfbc7

…ules Change-Id: I63accd3e881c941736ece4b4498c2c9d06ff8761 Signed-off-by: Stewart Smith <[email protected]>

error: the compiler can assume that the address of ‘r’ will never be …

f044df6

…NULL PRDF_ASSERT( &r != NULL ); Change-Id: I2d60075f9e2232512efe45a5c6aa5563f3a565f5 Signed-off-by: Stewart Smith <[email protected]>

error: in C++98 ‘l_vmVersionBuf’ must be initialized by constructor

ac29c7b

fix GCC6 thrown error Change-Id: I9aa508843f54c99ebb59822c39590811423ad0b1 Signed-off-by: Stewart Smith <[email protected]>

shenki mentioned this pull request Apr 16, 2020

Introduce an Alternate Toolchain option with gcc and binutils open-power/op-build#3564

Closed

paulmenzel reviewed Jun 17, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

P8 build fixes for modern GCC #193

P8 build fixes for modern GCC #193

shenki commented Apr 16, 2020

dcrowell77 commented Apr 22, 2020

shenki commented Apr 22, 2020

klauskiwi commented May 14, 2020

dcrowell77 commented May 15, 2020

paulmenzel Jun 17, 2020

shenki Jun 17, 2020

dcrowell77 Jun 17, 2020

P8 build fixes for modern GCC #193

Are you sure you want to change the base?

P8 build fixes for modern GCC #193

Conversation

shenki commented Apr 16, 2020

dcrowell77 commented Apr 22, 2020

shenki commented Apr 22, 2020

klauskiwi commented May 14, 2020

dcrowell77 commented May 15, 2020

paulmenzel Jun 17, 2020

Choose a reason for hiding this comment

shenki Jun 17, 2020

Choose a reason for hiding this comment

dcrowell77 Jun 17, 2020

Choose a reason for hiding this comment