On 8/31/22 13:39, Vineet Gupta wrote:


On 8/31/22 07:57, Palmer Dabbelt wrote:
   if (flag_pic)
-    riscv_cmodel = CM_PIC;
+    riscv_cmodel = CM_MEDANY;

   /* We get better code with explicit relocs for CM_MEDLOW, but
      worse code for the others (for now).  Pick the best default.  */

I'm fine either way on this one: having CM_PIC gone makes it a bit more likely to confuse CM_MEDANY with PIC, but flag_pic is overriding riscv_cmodel anyway so this isn't really used and deleting code is always a plus.

Indeed this was the most contentious part of removing CM_PIC, but it seems this is the way fwd. I'll add Kito's comment from [1] in code to make it more explicit.

[1]https://github.com/riscv-non-isa/riscv-c-api-doc/pull/11#issuecomment-686385585

I think I'll punt on this one, in the short-term. The reason being it affects USE_LOAD_ADDRESS_MACRO.

#define USE_LOAD_ADDRESS_MACRO(sym)                             \
  (!TARGET_EXPLICIT_RELOCS &&                                   \
   ((flag_pic                                                   \
     && ((SYMBOL_REF_P (sym) && SYMBOL_REF_LOCAL_P (sym))       \
         || ((GET_CODE (sym) == CONST)                          \
             && SYMBOL_REF_P (XEXP (XEXP (sym, 0),0))           \
             && SYMBOL_REF_LOCAL_P (XEXP (XEXP (sym, 0),0)))))  \
     || riscv_cmodel == CM_MEDANY))

With the patch, PIC implies CM_MEDANY and thus will change codegen for pic non-local symbols to also use the load address macro. I think we want to go in the opposite direction, i.e. wean away from the asm macros and have gcc codegen natively. It seems there are bugs in that area so once we flush them out (after creating a few as I don't know of any existing documented ones) this will get cleaned out.

Thx
-Vineet

Reply via email to