[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