[PATCH] D22990: [ELF] Implement R_ARM_TARGET1

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Target.cpp:1505
@@ -1504,1 +1504,3 @@
 
+static uint32_t modifyARMReloc(uint32_t Type) {
+  if (Type == R_ARM_TARGET1)
----------------
I think I'd prefer adding `case R_ARM_TARGET1` to the following switches because, by doing so, you don't need to read this function to understand how we handle ARM relocations for all relocation types except TARGET1. See below.

================
Comment at: ELF/Target.cpp:1536
@@ -1528,3 +1535,3 @@
     return R_GOT_PC;
   case R_ARM_TLS_GD32:
     return R_TLSGD_PC;
----------------
  case R_ARM_TARGET1:
    return Config->Target1Rel ? R_PC : R_ABS;

================
Comment at: ELF/Target.cpp:1558
@@ -1550,2 +1557,3 @@
 uint32_t ARMTargetInfo::getDynRel(uint32_t Type) const {
+  Type = modifyARMReloc(Type);
   if (Type == R_ARM_ABS32)
----------------
  if (Type == R_ARM_TARGET1 && !Config->Target1Rel)
    return R_ARM_ABS32;

================
Comment at: ELF/Target.cpp:1641
@@ -1631,3 +1640,3 @@
   case R_ARM_REL32:
   case R_ARM_TLS_GD32:
   case R_ARM_TLS_IE32:
----------------
Add `case R_ARM_TARGET32:` so that you can remove modifyARMReloc from this function.

================
Comment at: ELF/Target.cpp:1770
@@ -1759,3 +1769,3 @@
   case R_ARM_REL32:
   case R_ARM_TLS_GD32:
   case R_ARM_TLS_LDM32:
----------------
Ditto


https://reviews.llvm.org/D22990