Changeset View
Standalone View
share/mk/bsd.compiler.mk
| Show First 20 Lines • Show All 203 Lines • ▼ Show 20 Lines | .if ${${X_}COMPILER_TYPE} == "clang" || \ | ||||
| (${${X_}COMPILER_TYPE} == "gcc" && ${${X_}COMPILER_VERSION} >= 40800) | (${${X_}COMPILER_TYPE} == "gcc" && ${${X_}COMPILER_VERSION} >= 40800) | ||||
| ${X_}COMPILER_FEATURES+= c++11 | ${X_}COMPILER_FEATURES+= c++11 | ||||
| .endif | .endif | ||||
| .if ${${X_}COMPILER_TYPE} == "clang" && ${${X_}COMPILER_VERSION} >= 60000 | .if ${${X_}COMPILER_TYPE} == "clang" && ${${X_}COMPILER_VERSION} >= 60000 | ||||
| ${X_}COMPILER_FEATURES+= retpoline | ${X_}COMPILER_FEATURES+= retpoline | ||||
| .endif | .endif | ||||
| .else | .else | ||||
| # Use CC's values | # Use CC's values | ||||
| X_COMPILER_TYPE= ${COMPILER_TYPE} | ${X_}COMPILER_TYPE= ${COMPILER_TYPE} | ||||
bdrewery: This makes no sense now. `COMPILER_TYPE=${COMPILER_TYPE}`? | |||||
bdreweryUnsubmitted Not Done Inline ActionsIf it wasn't clear, that is the effect of your change. It achieves nothing that I can see. bdrewery: If it wasn't clear, that is the effect of your change. It achieves nothing that I can see. | |||||
jrtc27AuthorUnsubmitted Done Inline ActionsWe only reach this branch if ${cc} != "CC", ie ${X_} != "". jrtc27: We only reach this branch if `${cc} != "CC"`, ie `${X_} != ""`. | |||||
jrtc27AuthorUnsubmitted Done Inline ActionsIt is an NFC as far as FreeBSD upstream is concerned. However, downstreams exist that add additional variables to _cc_vars, and in that case ${X_} is neither empty nor "X_". Thus, whilst it's "pointless" upstream, it takes no effort to keep upstream and saves having to maintain it as a downstream patch and resolve conflicts every time someone adds a new COMPILER/LINKER_FOO. jrtc27: It is an NFC as far as FreeBSD upstream is concerned. However, downstreams exist that add… | |||||
jrtc27AuthorUnsubmitted Done Inline ActionsThis is also what the patch summary is trying to convey, in case it isn't clear. jrtc27: This is also what the patch summary is trying to convey, in case it isn't clear. | |||||
bdreweryUnsubmitted Not Done Inline ActionsIt's nonsense though and could easily be "fixed" by anyone since it's a bug/pointless to set a variable to itself. Especially without any comments or real reason. Just looking at this file I want to rewrite it since it's such chaos. Your change is easily lost if someone is motivated to do that. _cc_vars is not supposed to be modifiable. An underscore means that. I don't think it's good to commit hacks for private variables. Can you elaborate on what you have done exactly downstream? bdrewery: It's nonsense though and could easily be "fixed" by anyone since it's a bug/pointless to set a… | |||||
bdreweryUnsubmitted Not Done Inline ActionsAlso I'm not convinced that rS339636 is right which added these variables. If XLD/XCC is *not* set on the command line then that change may be wrong in recursive make. bdrewery: Also I'm not convinced that rS339636 is right which added these variables. If XLD/XCC is *not*… | |||||
bdreweryUnsubmitted Not Done Inline ActionsLooking deeper I think rS339636 is fine but it does leave a desire to refactor significantly. bdrewery: Looking deeper I think rS339636 is fine but it does leave a desire to refactor significantly. | |||||
jrtc27AuthorUnsubmitted Done Inline Actions
We're not setting a variable to itself; this case isn't hit when ${X_} is empty, so we will never be trying to execute COMPILER_TYPE=COMPILER_TYPE.
This is in CheriBSD; we've had _cc_vars+= CHERI_CC CHERI_ for years in our bsd.compiler.mk. jrtc27: > [..] since it's a bug/pointless to set a variable to itself [..]
We're not setting a… | |||||
jrtc27AuthorUnsubmitted Done Inline ActionsOk, turns out I'm wrong, _cc_vars+= CHERI_CC CHERI_ was added in September, in the precursor to the revision you mentioned. jrtc27: Ok, turns out I'm wrong, `_cc_vars+= CHERI_CC CHERI_` was added in September, in the precursor… | |||||
bdreweryUnsubmitted Not Done Inline ActionsYou want CHERI_COMPILER_TYPE to default to COMPILER_TYPE? CheriBSD is compiled using 3 different compilers and 3 different linkers? bdrewery: You want `CHERI_COMPILER_TYPE` to default to `COMPILER_TYPE`?
CheriBSD is compiled using 3… | |||||
jrtc27AuthorUnsubmitted Done Inline ActionsWell, yes, that is a current problem and should be fixed by just changing the if to ${cc} == "CC" || ${${cc}} != ${CC} (which is what the intent of the if *really* is). However, theoretically ${CHERI_CC} can be equal to ${CC} (could just use our built LLVM toolchain for both), which would then hit the else and still trash X_COMPILER_TYPE even if XCC is something different. As for why these exist, it's so pure MIPS stuff can still be built with GCC/binutils. It's quite possible we don't exercise that ability any more, but the flexibility in our build system is there and potentially useful. jrtc27: Well, yes, that is a current problem and should be fixed by just changing the `if` to `${cc} ==… | |||||
arichardsonUnsubmitted Not Done Inline ActionsWe use /use/bin/cc and /use/bin/LD for bootstrap/build tools. Until fairly recently we were using the bootstrapped GCC 4.2 + bfd as XCC/XLD and our forks of llvm/clang and lld for any code using CHERI capabilities (CHERI_CC/CHERI_LD) Some files are compiled with CHERI_CC instead of XCC during the libraries phase since we need to build e.g. a memcpy that works with CHERI capabilities. We then also have a buildcheri phase similar to lib32 that builds all libraries as using CHERI_CC. (It's actually slightly more complicated but hopefully this explains where we use the different compilers/linkers) arichardson: We use /use/bin/cc and /use/bin/LD for bootstrap/build tools. Until fairly recently we were… | |||||
| X_COMPILER_VERSION= ${COMPILER_VERSION} | ${X_}COMPILER_VERSION= ${COMPILER_VERSION} | ||||
| X_COMPILER_FREEBSD_VERSION= ${COMPILER_FREEBSD_VERSION} | ${X_}COMPILER_FREEBSD_VERSION= ${COMPILER_FREEBSD_VERSION} | ||||
| X_COMPILER_FEATURES= ${COMPILER_FEATURES} | ${X_}COMPILER_FEATURES= ${COMPILER_FEATURES} | ||||
| .endif # ${cc} == "CC" || (${cc} == "XCC" && ${XCC} != ${CC}) | .endif # ${cc} == "CC" || (${cc} == "XCC" && ${XCC} != ${CC}) | ||||
| # Export the values so sub-makes don't have to look them up again, using the | # Export the values so sub-makes don't have to look them up again, using the | ||||
| # hash key computed above. | # hash key computed above. | ||||
| .for var in ${_exported_vars} | .for var in ${_exported_vars} | ||||
| ${var}.${${X_}_cc_hash}:= ${${var}} | ${var}.${${X_}_cc_hash}:= ${${var}} | ||||
| .export-env ${var}.${${X_}_cc_hash} | .export-env ${var}.${${X_}_cc_hash} | ||||
| .undef ${var}.${${X_}_cc_hash} | .undef ${var}.${${X_}_cc_hash} | ||||
| Show All 10 Lines | |||||
This makes no sense now. COMPILER_TYPE=${COMPILER_TYPE}?