[PATCH] Emit the ARM build attributes ABI_PCS_wchar_t and ABI_enum_size.
Saleem Abdulrasool
compnerd at compnerd.org
Thu Jun 19 09:38:36 PDT 2014
Some minor nits, but LGTM. Thanks for adding support for this EABI attribute.
================
Comment at: include/llvm/Support/ARMBuildAttributes.h:162
@@ -161,1 +161,3 @@
+ // Tag_ABI_PCS_wchar_t
+ WCharProhibited = 0, // wchar_t is not used
----------------
Saleem Abdulrasool wrote:
> Can you add the id value in the comment much like the other ones please? And the same for the Tag_ABI_enum_size.
Can you add the uleb128 encoding and type similar to the other enums please?
================
Comment at: include/llvm/Support/ARMBuildAttributes.h:174
@@ -168,1 +173,3 @@
+ // Tag_ABI_enum_size
+ EnumProhibited = 0, // The user prohibited the use of enums when building
----------------
Same here.
================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:736
@@ +735,3 @@
+ // ABI_PCS_wchar_t to indicate wchar_t width
+ const ConstantInt *WCharWidthValue =
+ cast_or_null<ConstantInt>(SourceModule->getModuleFlag("short_wchar"));
----------------
You should be able to collapse the cast and if into a single statement.
I think you can use auto here without loss of clarity:
if (auto WCharType = cast_or_null<ConstantInt>(SM->getModuleFlag("short_wchar"))
================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:746
@@ +745,3 @@
+ // ABI_enum_size to indicate enum width
+ const ConstantInt *EnumWidthValue =
+ cast_or_null<ConstantInt>(SourceModule->getModuleFlag("short_enum"));
----------------
Similarly here.
================
Comment at: test/CodeGen/ARM/metadata-default.ll:4
@@ +3,3 @@
+; ModuleID = '-'
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-n32-S64"
+target triple = "armv7--none-eabi"
----------------
The Module ID doesnt add much to the test. You can trim the test to just the RUN line, the define, and metadata.
http://reviews.llvm.org/D4213
More information about the llvm-commits
mailing list