Page MenuHomePhabricator

[Support] Workaround a GCC 4.8 bug on constant expression evaluation.
ClosedPublic

Authored by hliao on Jul 30 2019, 7:50 AM.

Details

Summary
  • The following stripped code trigger a gcc-4.8 bug. To work that around, move the alignment evaluation into template parameter.
// https://godbolt.org/z/58p5_X
//
#include <cstddef>
#include <cstdint>

enum { aligned = 0, unaligned = 1 };

template <typename T, int alignment> struct PickAlignment {
  enum { value = alignment == 0 ? alignof(T) : alignment };
};

template <typename ValueType, std::size_t Alignment> struct packed {
private:
  struct {
    alignas(
        PickAlignment<ValueType, Alignment>::value) char buffer[sizeof(int)];
  } Value;
};

using ule16_t = packed<uint16_t, unaligned>;

ule16_t x;
  • Also, replace alignas with LLVMALIGN_AS to improve the compiler compatibility.

Diff Detail

Repository
rL LLVM

Event Timeline

hliao created this revision.Jul 30 2019, 7:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 30 2019, 7:50 AM

https://llvm.org/docs/GettingStarted.html#id7

GCC >=5.1.0 C/C++ compiler

The current recommendation is >=5.1.0...

hliao edited the summary of this revision. (Show Details)Jul 30 2019, 7:57 AM
hliao updated this revision to Diff 212349.Jul 30 2019, 7:58 AM

Format the commit message

hliao added a comment.Jul 30 2019, 7:59 AM

https://llvm.org/docs/GettingStarted.html#id7

GCC >=5.1.0 C/C++ compiler

The current recommendation is >=5.1.0...

But, unfortunately, there are many old systems with gcc-4.8 only, like ubuntu 16.04, centos 4.x, and other long-term systems

hliao added a comment.Jul 30 2019, 8:06 AM

BTW, if I read that correct, even though gcc 5.1 is listed as requirement, but that page also mentioned that gcc 4.8 could compile correctly the current LLVM codebase, https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library. As gcc 4.8.x is widely used in many long-term maintained systems, it would be nice to compile LLVM correctly against it.

Is it worth to workaround old gcc?

Support for gcc 4.8 will be dropped soon(ish) anyway..

hliao added a comment.Jul 30 2019, 8:59 AM

Is it worth to workaround old gcc?

Support for gcc 4.8 will be dropped soon(ish) anyway..

I don't want to be pedantic, but will means not right now. Many long-term systems, says CentOS 7.2 (with gcc-4.8.5 and the same issue), would be EOL up to 2024.

But one can use devtoolset on centos/rhel :)

There is a llvm-dev discusion (but now dead, Google blocked it? @jfb @chandlerc) about bumping gcc versions, LLVM wants to use C++-14.

Anyway, this small change looks fine.

xbolva00 accepted this revision.Jul 30 2019, 9:08 AM
This revision is now accepted and ready to land.Jul 30 2019, 9:08 AM
hliao added a comment.Jul 30 2019, 9:10 AM

But one can use devtoolset on centos/rhel :)

There is a llvm-dev discusion (but now dead, Google blocked it? @jfb @chandlerc) about bumping gcc versions, LLVM wants to use C++-14.

Anyway, this small change looks fine.

thanks so much!

This revision was automatically updated to reflect the committed changes.
jfb added a comment.Jul 30 2019, 9:26 AM

But one can use devtoolset on centos/rhel :)

There is a llvm-dev discusion (but now dead, Google blocked it? @jfb @chandlerc) about bumping gcc versions, LLVM wants to use C++-14.

Anyway, this small change looks fine.

It should happen soon indeed. It was supposed to be months ago, but things slipped a bit. The change is fine since we do still kinda support GCC 4.8 for now, though I think we should remove LLVM_ALIGNAS (I'll do so...).