MaskRay marked an inline comment as done.
MaskRay added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13318-13320
+ Value *Idxs[] = {Builder.getInt32(0), Builder.getInt32(i - 1)};
+ Value *Features = Builder.CreateAlignedLoad(
+ Int32Ty, Builder.CreateGEP(ATy, CpuFeatures2, Idxs),
----------------
MaskRay wrote:
> pengfei wrote:
> > MaskRay wrote:
> > > pengfei wrote:
> > > > Do we have problem when linking with old version of libgcc?
> > > If we don't use `__cpu_features2[1..3]`, no problem with older libgcc.
> > >
> > > The way GCC upgraded `__cpu_features2` to an array is compatible with
> > > scalar `__cpu_features2`.
> > I assume GCC seldom links to older libgcc, but there's no guarantee Clang
> > can link to latest libgcc.
> > And we cannot assume user won't use it once the method provided.
> > I don't have a good idea for it, but I think we should write the
> > requirement done in release note or somewhere if we cannot find a better
> > way.
> The way we treat `__cpu_features2` as an array is compatible with older
> libgcc providing scalar `__cpu_features2`, as long as we don't access the
> high bits (`FEATURE_X86_64_{BASELINE,V2,V3,V4}`).
>
> * When `arch=x86-64` is used, this patch changes a compiler crash to (if
> newer libgcc, work; if older libgcc, runtime crash)
> * When `arch=x86-64*` is unused, the same as before.
I think a release note isn't really necessary as the compatibility story with
this patch is strictly better than the current state.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158329/new/
https://reviews.llvm.org/D158329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
