Xiangling_L added inline comments.
================
Comment at: clang/docs/ClangCommandLineReference.rst:2868
+Only supported On AIX. Specify usage of volatile and nonvolatile vector
registers, the extended vector ABI on AIX. Defaults to '-mnovecnvol' when
Altivec is enabled.
+
----------------
s/On/on;
================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:532
Options.EmitCallSiteInfo = CodeGenOpts.EmitCallSiteInfo;
+ Options.AIXExtendedAltivecABI = CodeGenOpts.AIXExtendedAltivecABI;
Options.ValueTrackingVariableLocations =
----------------
The ABI specifies `When the option to use nonvolatile vector registers is
enalbed. the compilation environment must also predefine __EXTABI__`. I didn't
see this. Should we also cover this in this patch?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4571
}
+ if (Arg *A =
----------------
On clang, when we do:
`clang -target powerpc-ibm-aix-xcoff -maltivec -S -emit-llvm test_faltivec.c`,
clang driver passes `-target-cpu pwr4` as default arch to frontend without
issuing any error.
However, with XL, we have:
`"-qaltivec" is not compatible with "-qarch=pwr4". "-qnoaltivec" is being set.`
The same error will be issued if `pwr5` is used as well.
So I suppose for AIX in clang, when user use `-maltivec` without specifying
arch level, we can do:
1) by default pass `-target-cpu pwr6` to frontend
or 2) issue error for "-qarch=pwr4"+ enable altivec
or 3) issue error for `-qacrh = pwr4` + diable altivec like XL does?
Also we should emit error when user use `-maltivec` with -mcpu=pwr5.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4579
+
+ bool haveMaltivec = false;
+
----------------
I would suggest `s/haveMaltivec/HasAltivec` to be consistent with other places
where if altivec enabled is tested.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4581
+
+ for (const Arg *A : Args) {
+ auto optID = A->getOption().getID();
----------------
Any reason why we cannot use `Args.getLastArg` here for `OPT_maltivec` as well?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4591
+
+ if (A->getOption().matches(options::OPT_mnovecnvol) && haveMaltivec)
+ D.Diag(diag::err_aix_default_altivec_abi);
----------------
Since we are defaulting to default altivec ABI, so I think the logic here
should be if (HasAltivec && !Args.getLastArg(options::OPT_mvecnvol)), then we
emit `D.Diag(diag::err_aix_default_altivec_abi)` error?
================
Comment at: clang/test/CodeGen/altivec.c:53
}
+
+// AIX-ERROR: error: The default Altivec ABI on AIX is not yet supported, use
'-mvecnvol' for the extended Altivec ABI
----------------
Could we also add a testcase to test `-mvecnvol/-mnovecnvol` are AIX only
options?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89684/new/
https://reviews.llvm.org/D89684
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
