Extend GCC workaround to GCC < 8.4 for llvm::iterator_range ctor #82643
Conversation
GCC SFINAE error with decltype was fixed in commit ac5e28911abdfb8d9bf6bea980223e199bbcf28d which made it into GCC 8.4. Therefore adjust GCC version test accordingly.
|
@llvm/pr-subscribers-llvm-adt Author: Thomas Preud'homme (RoboTux) ChangesGCC SFINAE error with decltype was fixed in commit Full diff: https://github.com/llvm/llvm-project/pull/82643.diff 1 Files Affected:
diff --git a/llvm/include/llvm/ADT/iterator_range.h b/llvm/include/llvm/ADT/iterator_range.h
index 2dc227935984b1..7d288ea4506ba5 100644
--- a/llvm/include/llvm/ADT/iterator_range.h
+++ b/llvm/include/llvm/ADT/iterator_range.h
@@ -43,8 +43,8 @@ class iterator_range {
IteratorT begin_iterator, end_iterator;
public:
-#if __GNUC__ == 7
- // Be careful no to break gcc-7 on the mlir target.
+#if __GNUC__ == 7 || (__GNUC__ == 8 && __GNUC_MINOR__ < 4)
+ // Be careful no to break gcc-7 and gcc-8 < 8.4 on the mlir target.
// See https://github.com/llvm/llvm-project/issues/63843
template <typename Container>
#else
|
RoboTux
requested review from
MaskRay,
joker-eph,
dwblaikie and
s-barannikov
|
Is this https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87748 (8.x regression)? 8.4 is the fourth minor release in the 8.x series. I think it's pretty unfortunate that the issue was noticed and fixed late in the 8.x series. That said, 8.x is also quite old in 2024 and perhaps newer major releases have been better at catching regressions?. @tstellar Is there anything a distribution can help to catch such regressions early? |
At least for Fedora, we've switched to building clang with clang, so it would be hard to catch this in distribution testing. Maybe we need some more buildbots to test the versions of gcc we support. |
Yes it is.
FYI GCC 8.3 is what Debian Buster is using. It's in LTS support so is only seeing security related updates, hence why it is not using GCC 8.4. |
|
/cherry-pick 7f71fa9 |
|
/pull-request #82688 |
It's difficult to be comprehensive: for example I have a bot for gcc 7.5, but this kind of fix made it to gcc 8.4 and was back ported to gcc 7.4. |
|
We could also change our policy to only support the latest minor release of gcc-X. That would make it much easier to test. |
|
Have you tested on GCC 8.3.0? Even after applying this patch, I'm still getting compilation errors. The error message is as follows: /home/dongshuai/test/llvm-project/mlir/lib/Transforms/Utils/CFGToSCF.cpp: In function ‘mlir::FailureOr<llvm::SmallVectormlir::Block* > transformToStructuredCFBranches(mlir::Block*, mlir::function_ref<mlir::Value(unsigned int)>, mlir::function_refmlir::Value(mlir::Type), mlir::CFGToSCFInterface&, mlir::DominanceInfo&)’: |
I had tested a build of my project that uses MLIR with GCC 8.3 (built from the corresponding git release branch) and it built fine (it wasn't without this patch). I've just tried building just MLIR and I'm seeing a similar error. |
|
Alright, found a fix that seems to work. Bear with me while I do a full mlir build. |
|
Nope sorry, it fixed one issue but there are more cases of decltype involved in SFINAE and it's hard to fix all of them. The current patch works for some users of MLIR at least, though clearly not all of them since building all of MLIR doesn't. If people don't mind I'd like to keep the current patch in while we support GCC 8 because it helps my usecase and doesn't break any build not already broken (if it does I'm happy to revert it myself). |
|
@loongknown The patch at RoboTux@8d161cb fixes the upstream build failures when compiling with GCC 8.3 for me. Can you confirm me it works for you and I'll make a PR. |
Yes, it works. Tank you! |
Great, I've created #83266 |
GCC SFINAE error with decltype was fixed in commit
ac5e28911abdfb8d9bf6bea980223e199bbcf28d which made it into GCC 8.4.
Therefore adjust GCC version test accordingly.