[PATCH] Add ARM big endian Target (armeb, thumbeb)

Tim Northover t.p.northover at gmail.com
Thu Mar 20 03:36:04 PDT 2014

  Hi Christian,

  > This function returns the full instruction size of the fixed-up instruction,
  > and is not the number of bytes to fixup.

  Ok, that brings up two questions:

  Did you look into using the DataSize variable? Perhaps more than one instruction ends up in the same block, in which case it wouldn't work. An explanation in comments would be good if so.

  What makes you think that FK_Data_1 ends up attached to an instruction needing a 2-byte fixup? At the very least this needs commenting and tests (like everything else) since it's a surprising fact.



Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:719
@@ -663,3 +718,3 @@
   ELFARMAsmBackend(const Target &T, const StringRef TT,
-                   uint8_t _OSABI)
-    : ARMAsmBackend(T, TT), OSABI(_OSABI) { }
+                   uint8_t _OSABI, bool _IsLittle)
+    : ARMAsmBackend(T, TT, _IsLittle), OSABI(_OSABI) { }
Identifiers beginning with an underscore and a capital letter are reserved in C++. Using them is undefined behaviour.

Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:790
@@ +789,3 @@
+MCAsmBackend *llvm::createThumbleAsmBackend(const Target &T,
+                                          const MCRegisterInfo &MRI,
"Thumble" looks rather like an actual word, and "Thumbbe" like a 17th century typo. I'd probably capitalise the BE/LE.

Comment at: lib/Target/ARM/ARMTargetMachine.h:171
@@ +170,3 @@
+class ThumbleTargetMachine : public ThumbTargetMachine {
+  virtual void anchor();
Are these extra classes needed for some weird magic hidden elsewhere? Because they look fairly cluttered for the sake of one parameter in a constructor.


More information about the llvm-commits mailing list