[PATCH] D44963: ELF: Add support for short thunks on ARM.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Generally looking good.



================
Comment at: lld/ELF/Thunks.cpp:67
 
+class ARMThunk : public Thunk {
+public:
----------------
It would be nice to explain as a comment about why we have more than one thunk for ARM.


================
Comment at: lld/ELF/Thunks.cpp:73-76
+  bool isCompatibleWith(RelType Type) const override;
+
+  virtual uint32_t sizeLong() const = 0;
+  virtual void writeLong(uint8_t *Buf) const = 0;
----------------
Could you add comments for these new member functions?


================
Comment at: lld/ELF/Thunks.cpp:234
 
-void ARMV7ABSLongThunk::writeTo(uint8_t *Buf) const {
+bool ARMThunk::mayUseShortThunk() const {
+  uint64_t S = getARMThunkDestVA(Destination);
----------------
I'd add a comment saying that this function returns true if the target is not Thumb and is within 2^26.


https://reviews.llvm.org/D44963