diff mbox series

RISC-V: make USE_LOAD_ADDRESS_MACRO easier to understand

Message ID 20220902230820.381916-1-vineetg@rivosinc.com
State New, archived
Headers
Series RISC-V: make USE_LOAD_ADDRESS_MACRO easier to understand |

Commit Message

Vineet Gupta Sept. 2, 2022, 11:08 p.m. UTC 
  The current macro has several && and || making it really hard to understand
the first time.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
Since we are on this topic, perhaps get this simplification too.

But I'm not sure if the current checking of local symbol can be simplified
a bit. Isn't the first line enough for GET_CODE == const case too ?

---
 gcc/config/riscv/riscv.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Kito Cheng Sept. 23, 2022, 4:01 p.m. UTC | #1 
Committed with ChangeLog and minor naming tweaking.


> But I'm not sure if the current checking of local symbol can be simplified
> a bit. Isn't the first line enough for GET_CODE == const case too ?

SYMBOL_REF_P not work for CONST, SYMBOL_REF_P is just checking
GET_CODE is SYMBOL_REF, and SYMBOL_REF_LOCAL_P will also ICE if you
feed something other than SYMBOL_REF when checking enabled...

On Sat, Sep 3, 2022 at 7:08 AM Vineet Gupta <vineetg@rivosinc.com> wrote:
>
> The current macro has several && and || making it really hard to understand
> the first time.
>
> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> ---
> Since we are on this topic, perhaps get this simplification too.
>
> But I'm not sure if the current checking of local symbol can be simplified
> a bit. Isn't the first line enough for GET_CODE == const case too ?
>
> ---
>  gcc/config/riscv/riscv.h | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index eb1284e56d69..3e3f67ef8270 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -749,18 +749,19 @@ typedef struct {
>  #define CASE_VECTOR_MODE SImode
>  #define CASE_VECTOR_PC_RELATIVE (riscv_cmodel != CM_MEDLOW)
>
> +#define LOCAL_SYM(sym)                                                 \
> +     ((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))))
> +
>  /* The load-address macro is used for PC-relative addressing of symbols
>     that bind locally.  Don't use it for symbols that should be addressed
>     via the GOT.  Also, avoid it for CM_MEDLOW, where LUI addressing
>     currently results in more opportunities for linker relaxation.  */
>  #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))
> +   ((flag_pic && LOCAL_SYM(sym)) || riscv_cmodel == CM_MEDANY))
>
>  /* Define this as 1 if `char' should by default be signed; else as 0.  */
>  #define DEFAULT_SIGNED_CHAR 0
> --
> 2.32.0
>
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index eb1284e56d69..3e3f67ef8270 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -749,18 +749,19 @@  typedef struct {
 #define CASE_VECTOR_MODE SImode
 #define CASE_VECTOR_PC_RELATIVE (riscv_cmodel != CM_MEDLOW)
 
+#define LOCAL_SYM(sym)							\
+     ((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))))
+
 /* The load-address macro is used for PC-relative addressing of symbols
    that bind locally.  Don't use it for symbols that should be addressed
    via the GOT.  Also, avoid it for CM_MEDLOW, where LUI addressing
    currently results in more opportunities for linker relaxation.  */
 #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))
+   ((flag_pic && LOCAL_SYM(sym)) || riscv_cmodel == CM_MEDANY))
 
 /* Define this as 1 if `char' should by default be signed; else as 0.  */
 #define DEFAULT_SIGNED_CHAR 0