* [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off
@ 2023-11-30 18:39 Taylor Simpson
2023-11-30 19:33 ` Brian Cain
2023-11-30 20:16 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 5+ messages in thread
From: Taylor Simpson @ 2023-11-30 18:39 UTC (permalink / raw)
To: qemu-devel
Cc: bcain, quic_mathbern, sidneym, quic_mliebel, richard.henderson,
philmd, ale, anjo, ltaylorsimpson
Adding -Werror=shadow=compatible-local causes Hexagon not to build
when idef-parser is off. The "label" variable in CHECK_NOSHUF_PRED
shadows a variable in the surrounding code.
Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
---
target/hexagon/macros.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 9a51b5709b..f99390e2a8 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -93,13 +93,13 @@
#define CHECK_NOSHUF_PRED(GET_EA, SIZE, PRED) \
do { \
- TCGLabel *label = gen_new_label(); \
- tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, label); \
+ TCGLabel *noshuf_label = gen_new_label(); \
+ tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
GET_EA; \
if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
probe_noshuf_load(EA, SIZE, ctx->mem_idx); \
} \
- gen_set_label(label); \
+ gen_set_label(noshuf_label); \
if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
process_store(ctx, 1); \
} \
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* RE: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off
2023-11-30 18:39 [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off Taylor Simpson
@ 2023-11-30 19:33 ` Brian Cain
2023-11-30 20:16 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 5+ messages in thread
From: Brian Cain @ 2023-11-30 19:33 UTC (permalink / raw)
To: Taylor Simpson, qemu-devel@nongnu.org
Cc: Matheus Bernardino (QUIC), Sid Manning, Marco Liebel (QUIC),
richard.henderson@linaro.org, philmd@linaro.org, ale@rev.ng,
anjo@rev.ng
> -----Original Message-----
> From: Taylor Simpson <ltaylorsimpson@gmail.com>
> Sent: Thursday, November 30, 2023 12:40 PM
> To: qemu-devel@nongnu.org
> Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>; Marco
> Liebel (QUIC) <quic_mliebel@quicinc.com>; richard.henderson@linaro.org;
> philmd@linaro.org; ale@rev.ng; anjo@rev.ng; ltaylorsimpson@gmail.com
> Subject: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-
> parser is off
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> Adding -Werror=shadow=compatible-local causes Hexagon not to build
> when idef-parser is off. The "label" variable in CHECK_NOSHUF_PRED
> shadows a variable in the surrounding code.
>
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
> target/hexagon/macros.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> index 9a51b5709b..f99390e2a8 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -93,13 +93,13 @@
>
> #define CHECK_NOSHUF_PRED(GET_EA, SIZE, PRED) \
> do { \
> - TCGLabel *label = gen_new_label(); \
> - tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, label); \
> + TCGLabel *noshuf_label = gen_new_label(); \
> + tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
> GET_EA; \
> if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
> probe_noshuf_load(EA, SIZE, ctx->mem_idx); \
> } \
> - gen_set_label(label); \
> + gen_set_label(noshuf_label); \
> if (insn->slot == 0 && ctx->pkt->pkt_has_store_s1) { \
> process_store(ctx, 1); \
> } \
> --
> 2.34.1
Reviewed-by: Brian Cain <bcain@quicinc.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off
2023-11-30 18:39 [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off Taylor Simpson
2023-11-30 19:33 ` Brian Cain
@ 2023-11-30 20:16 ` Philippe Mathieu-Daudé
2023-11-30 20:39 ` Brian Cain
1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-11-30 20:16 UTC (permalink / raw)
To: Taylor Simpson, qemu-devel
Cc: bcain, quic_mathbern, sidneym, quic_mliebel, richard.henderson,
ale, anjo
On 30/11/23 19:39, Taylor Simpson wrote:
> Adding -Werror=shadow=compatible-local causes Hexagon not to build
> when idef-parser is off. The "label" variable in CHECK_NOSHUF_PRED
> shadows a variable in the surrounding code.
>
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
> target/hexagon/macros.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> index 9a51b5709b..f99390e2a8 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -93,13 +93,13 @@
>
> #define CHECK_NOSHUF_PRED(GET_EA, SIZE, PRED) \
> do { \
> - TCGLabel *label = gen_new_label(); \
> - tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, label); \
> + TCGLabel *noshuf_label = gen_new_label(); \
> + tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
Fragile, but sufficient.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off
2023-11-30 20:16 ` Philippe Mathieu-Daudé
@ 2023-11-30 20:39 ` Brian Cain
2023-12-01 10:50 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Brian Cain @ 2023-11-30 20:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Taylor Simpson,
qemu-devel@nongnu.org
Cc: Matheus Bernardino (QUIC), Sid Manning, Marco Liebel (QUIC),
richard.henderson@linaro.org, ale@rev.ng, anjo@rev.ng
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Thursday, November 30, 2023 2:17 PM
> To: Taylor Simpson <ltaylorsimpson@gmail.com>; qemu-devel@nongnu.org
> Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>; Marco
> Liebel (QUIC) <quic_mliebel@quicinc.com>; richard.henderson@linaro.org;
> ale@rev.ng; anjo@rev.ng
> Subject: Re: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-
> parser is off
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> On 30/11/23 19:39, Taylor Simpson wrote:
> > Adding -Werror=shadow=compatible-local causes Hexagon not to build
> > when idef-parser is off. The "label" variable in CHECK_NOSHUF_PRED
> > shadows a variable in the surrounding code.
> >
> > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> > ---
> > target/hexagon/macros.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> > index 9a51b5709b..f99390e2a8 100644
> > --- a/target/hexagon/macros.h
> > +++ b/target/hexagon/macros.h
> > @@ -93,13 +93,13 @@
> >
> > #define CHECK_NOSHUF_PRED(GET_EA, SIZE, PRED) \
> > do { \
> > - TCGLabel *label = gen_new_label(); \
> > - tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, label); \
> > + TCGLabel *noshuf_label = gen_new_label(); \
> > + tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
>
> Fragile, but sufficient.
The fragility here refers to the fact that CHECK_NOSHUF_PRED() macro could show up in other contexts and then could shadow those?
We could change the macro to a function or expand the macro to take a label declared outside. Would that be preferred? Or are there other suggestions?
-Brian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off
2023-11-30 20:39 ` Brian Cain
@ 2023-12-01 10:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-12-01 10:50 UTC (permalink / raw)
To: Brian Cain, Taylor Simpson, qemu-devel@nongnu.org
Cc: Matheus Bernardino (QUIC), Sid Manning, Marco Liebel (QUIC),
richard.henderson@linaro.org, ale@rev.ng, anjo@rev.ng
On 30/11/23 21:39, Brian Cain wrote:
>
>
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Sent: Thursday, November 30, 2023 2:17 PM
>> To: Taylor Simpson <ltaylorsimpson@gmail.com>; qemu-devel@nongnu.org
>> Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
>> <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>; Marco
>> Liebel (QUIC) <quic_mliebel@quicinc.com>; richard.henderson@linaro.org;
>> ale@rev.ng; anjo@rev.ng
>> Subject: Re: [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-
>> parser is off
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary of
>> any links or attachments, and do not enable macros.
>>
>> On 30/11/23 19:39, Taylor Simpson wrote:
>>> Adding -Werror=shadow=compatible-local causes Hexagon not to build
>>> when idef-parser is off. The "label" variable in CHECK_NOSHUF_PRED
>>> shadows a variable in the surrounding code.
>>>
>>> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
>>> ---
>>> target/hexagon/macros.h | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
>>> index 9a51b5709b..f99390e2a8 100644
>>> --- a/target/hexagon/macros.h
>>> +++ b/target/hexagon/macros.h
>>> @@ -93,13 +93,13 @@
>>>
>>> #define CHECK_NOSHUF_PRED(GET_EA, SIZE, PRED) \
>>> do { \
>>> - TCGLabel *label = gen_new_label(); \
>>> - tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, label); \
>>> + TCGLabel *noshuf_label = gen_new_label(); \
>>> + tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
>>
>> Fragile, but sufficient.
>
> The fragility here refers to the fact that CHECK_NOSHUF_PRED() macro could show up in other contexts and then could shadow those?
Yes.
> We could change the macro to a function or expand the macro to take a label declared outside. Would that be preferred? Or are there other suggestions?
Nah, this is good enough, no need to over-engineer IMHO
(I just wanted to remark this could still bit us in the
future, and -Werror will fire).
Regards,
Phil.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-01 10:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-30 18:39 [PATCH] Hexagon (target/hexagon) Fix shadow variable when idef-parser is off Taylor Simpson
2023-11-30 19:33 ` Brian Cain
2023-11-30 20:16 ` Philippe Mathieu-Daudé
2023-11-30 20:39 ` Brian Cain
2023-12-01 10:50 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).