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), ---------------- 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. ================ Comment at: compiler-rt/lib/builtins/cpu_model.c:167 + FEATURE_WP, + FEATURE_LZCNT = 57, + FEATURE_MOVBE, ---------------- FreddyYe wrote: > ` = 57` redundant Thanks. Minor issue, I've fixed it locally and will only upload a new diff if other things need changing. 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