| Message ID | 55ECFE08.8060405@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi, On Mon, 7 Sep 2015, Kugan wrote: > Allow GIMPLE_DEBUG with values in promoted register. Patch does much more. > gcc/ChangeLog: > > 2015-09-07 Kugan Vivekanandarajah <kuganv@linaro.org> > > * expr.c (expand_expr_real_1): Set proper SUNREG_PROMOTED_MODE for > SSA_NAME that was set by GIMPLE_CALL and assigned to another > SSA_NAME of same type. ChangeLog doesn't match patch, and patch contains dubious changes: > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -5240,7 +5240,6 @@ expand_debug_locations (void) > tree value = (tree)INSN_VAR_LOCATION_LOC (insn); > rtx val; > rtx_insn *prev_insn, *insn2; > - machine_mode mode; > > if (value == NULL_TREE) > val = NULL_RTX; > @@ -5275,16 +5274,6 @@ expand_debug_locations (void) > > if (!val) > val = gen_rtx_UNKNOWN_VAR_LOC (); > - else > - { > - mode = GET_MODE (INSN_VAR_LOCATION (insn)); > - > - gcc_assert (mode == GET_MODE (val) > - || (GET_MODE (val) == VOIDmode > - && (CONST_SCALAR_INT_P (val) > - || GET_CODE (val) == CONST_FIXED > - || GET_CODE (val) == LABEL_REF))); > - } > > INSN_VAR_LOCATION_LOC (insn) = val; > prev_insn = PREV_INSN (insn); So it seems that the modes of the values location and the value itself don't have to match anymore, which seems dubious when considering how a debugger should load the value in question from the given location. So, how is it supposed to work? And this change: > --- a/gcc/rtl.h > +++ b/gcc/rtl.h > @@ -2100,6 +2100,8 @@ wi::int_traits <rtx_mode_t>::decompose (HOST_WIDE_INT*, > targets is 1 rather than -1. */ > gcc_checking_assert (INTVAL (x.first) > == sext_hwi (INTVAL (x.first), precision) > + || INTVAL (x.first) > + == (INTVAL (x.first) & ((1 << precision) - 1)) > || (x.second == BImode && INTVAL (x.first) == 1)); > > return wi::storage_ref (&INTVAL (x.first), 1, precision); implies that wide_ints are not always sign-extended anymore after you changes. That's a fundamental assumption, so removing that assert implies that you somehow created non-canonical wide_ints, and those will cause bugs elsewhere in the code. Don't just remove asserts, they are usually there for a reason, and without accompanying changes those reasons don't go away. Ciao, Michael.
From a28de63bcbb9f315cee7e41be11b65b3ff521a91 Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> Date: Tue, 1 Sep 2015 08:40:40 +1000 Subject: [PATCH 5/8] debug stmt in widen mode --- gcc/cfgexpand.c | 11 ----------- gcc/gimple-ssa-type-promote.c | 7 ------- gcc/rtl.h | 2 ++ 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index bbc3c10..036085a 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5240,7 +5240,6 @@ expand_debug_locations (void) tree value = (tree)INSN_VAR_LOCATION_LOC (insn); rtx val; rtx_insn *prev_insn, *insn2; - machine_mode mode; if (value == NULL_TREE) val = NULL_RTX; @@ -5275,16 +5274,6 @@ expand_debug_locations (void) if (!val) val = gen_rtx_UNKNOWN_VAR_LOC (); - else - { - mode = GET_MODE (INSN_VAR_LOCATION (insn)); - - gcc_assert (mode == GET_MODE (val) - || (GET_MODE (val) == VOIDmode - && (CONST_SCALAR_INT_P (val) - || GET_CODE (val) == CONST_FIXED - || GET_CODE (val) == LABEL_REF))); - } INSN_VAR_LOCATION_LOC (insn) = val; prev_insn = PREV_INSN (insn); diff --git a/gcc/gimple-ssa-type-promote.c b/gcc/gimple-ssa-type-promote.c index 62b5fdc..6805b9c 100644 --- a/gcc/gimple-ssa-type-promote.c +++ b/gcc/gimple-ssa-type-promote.c @@ -570,13 +570,6 @@ fixup_uses (tree use, tree promoted_type, tree old_type) bool do_not_promote = false; switch (gimple_code (stmt)) { - case GIMPLE_DEBUG: - { - gsi = gsi_for_stmt (stmt); - gsi_remove (&gsi, true); - break; - } - case GIMPLE_ASM: case GIMPLE_CALL: case GIMPLE_RETURN: diff --git a/gcc/rtl.h b/gcc/rtl.h index ac56133..c3cdf96 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -2100,6 +2100,8 @@ wi::int_traits <rtx_mode_t>::decompose (HOST_WIDE_INT *, targets is 1 rather than -1. */ gcc_checking_assert (INTVAL (x.first) == sext_hwi (INTVAL (x.first), precision) + || INTVAL (x.first) + == (INTVAL (x.first) & ((1 << precision) - 1)) || (x.second == BImode && INTVAL (x.first) == 1)); return wi::storage_ref (&INTVAL (x.first), 1, precision); -- 1.9.1