pengfei added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2581
+ unsigned VectorWidth) {
+ if (!getTarget().getTriple().isX86() || VectorWidth < 512)
+ return;
----------------
skan wrote:
> Minor suggestion. The code here may be refined to be better extended by other
> targets, like
> ```
> llvm::Triple::ArchType ArchType =
> getContext().getTargetInfo().getTriple().getArch();
>
> switch (ArchType) {
> case llvm::Triple::x86:
> case llvm::Triple::x86_64: {
> ....
> }
> default:
> return;
>
> ```
We have a few place code using `isX86`. I think it's more convenient to use a
single condition. We can refactor when necessary.
================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:927
+ !STI.hasFeature(X86::FeatureAVX10_512bit))
+ report_fatal_error("ZMM registers are not supported without AVX10-512BIT");
switch (TSFlags & X86II::OpPrefixMask) {
----------------
skan wrote:
> skan wrote:
> > -mavx10.1 does not work for assembler. So if such instruction is generated
> > w/o AVX10-512BIT support, it must be compiler's issue instead of user's. An
> > `assert` should be more appropriate here.
> > -mavx10.1 does not work for assembler. So if such instruction is generated
> > w/o AVX10-512BIT support, it must be compiler's issue instead of user's. An
> > `assert` should be more appropriate here.
>
> Reference: https://llvm.org/docs/CodingStandards.html#assert-liberally
We need to report fatal error for this case even if it's a compiler bug.
Otherwise, user may observe the crash issue in runtime and hard to find the
reason.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157485/new/
https://reviews.llvm.org/D157485
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
