[PATCH] D50141: Add errors for tiny codemodel on targets other than AArch64

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: include/llvm/Target/TargetMachine.h:359
+getEffectiveCodeModel(Optional<CodeModel::Model> CM,
+                      CodeModel::Model Default = CodeModel::Small) {
+  if (CM) {
----------------
peter.smith wrote:
> I have a mild personal preference not to use a default parameter here, in other words pass in default code model for all callers and not just Lanai and WebAssembly. CodeModel::Small is by far the most common but I think it makes it easier to see what the default is from the individual backend without having to go up a few levels.
> 
> Not a strong opinion, so feel free to ignore.
Yeah, I agree. That sounds sensible.


================
Comment at: include/llvm/Target/TargetMachine.h:361
+  if (CM) {
+    if (*CM == CodeModel::Tiny)
+      report_fatal_error("Target does not support the Tiny CodeModel");
----------------
peter.smith wrote:
> Is it worth checking that the Kernel code model is not supported as well? As far as I can tell this is only supported on AArch64. Admittedly it looks like the chances of someone using that by mistake is much lower than CodeModel::Tiny 
Yep. As far as I can tell it may do something on X86 and AArch64. The others it is ignored on or treated the same as the Small model.


https://reviews.llvm.org/D50141