tlively added inline comments.
================
Comment at: lib/Basic/Targets/WebAssembly.cpp:89
+ // features control availability of builtins
+ setSIMDLevel(Features, SIMDLevel);
+ if (HasNontrappingFPToInt)
----------------
aheejin wrote:
> Minor thing, but should we extract this as a function? It is gonna be a
> couple line anyway, like
>
> ```
> if (CPU == "bleeding-edge") {
> ...
> Features["unimplemented-simd128"] = Features["simd128"] = true;
> }
>
> if (SIMDLevel >= SIMD128)
> Features["simd128"] = true;
> if (SIMDLevel >= UnimplementedSIMD128)
> Features["unimplemented-simd128"] = true;
> ...
>
> And to me it is more readable to see all `Features` setting in one place. But
> I'm not too opinionated either.
The structure of basically all this code is pulled from X86.cpp, which is
obviously has a lot more features to wrangle. This particular function is
similar to `setSSELevel` in X86.cpp. I agree that it probably doesn't need to
be separate now, but as we explore possible extensions to the SIMD proposal in
the future I think it will be useful to have this function.
================
Comment at: lib/Basic/Targets/WebAssembly.cpp:98
+ return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
+ }
+
----------------
aheejin wrote:
> The indentation of these functions looks weird and there are lines that
> exceeds 80 cols. clang-format?
Done, and copied my pre-commit git hooks from the main LLVM repo so it won't be
an issue again.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56504/new/
https://reviews.llvm.org/D56504
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
