| Message ID | 1445376433-14658-9-git-send-email-rth@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/20/2015 03:27 PM, Richard Henderson wrote: > --- > gcc/config/i386/i386.c | 10 ++++++++++ > gcc/doc/tm.texi | 5 +++++ > gcc/doc/tm.texi.in | 2 ++ > gcc/fold-const.c | 6 +++++- > gcc/gimple.c | 12 +++++++++--- > gcc/target.def | 9 +++++++++ > gcc/targhooks.c | 9 +++++++++ > gcc/targhooks.h | 1 + > 8 files changed, 50 insertions(+), 4 deletions(-) Seems reasonable. Needs ChangeLog. Tests would be good too. jeff
On 10/21/2015 03:57 AM, Jeff Law wrote: > On 10/20/2015 03:27 PM, Richard Henderson wrote: >> --- >> gcc/config/i386/i386.c | 10 ++++++++++ >> gcc/doc/tm.texi | 5 +++++ >> gcc/doc/tm.texi.in | 2 ++ >> gcc/fold-const.c | 6 +++++- >> gcc/gimple.c | 12 +++++++++--- >> gcc/target.def | 9 +++++++++ >> gcc/targhooks.c | 9 +++++++++ >> gcc/targhooks.h | 1 + >> 8 files changed, 50 insertions(+), 4 deletions(-) > Seems reasonable. Needs ChangeLog. Tests would be good too. The test got split to patch 11. I don't recall if the test can pass without patches 9-10 as well. r~
On 10/21/2015 02:50 PM, Richard Henderson wrote: > On 10/21/2015 03:57 AM, Jeff Law wrote: >> On 10/20/2015 03:27 PM, Richard Henderson wrote: >>> --- >>> gcc/config/i386/i386.c | 10 ++++++++++ >>> gcc/doc/tm.texi | 5 +++++ >>> gcc/doc/tm.texi.in | 2 ++ >>> gcc/fold-const.c | 6 +++++- >>> gcc/gimple.c | 12 +++++++++--- >>> gcc/target.def | 9 +++++++++ >>> gcc/targhooks.c | 9 +++++++++ >>> gcc/targhooks.h | 1 + >>> 8 files changed, 50 insertions(+), 4 deletions(-) >> Seems reasonable. Needs ChangeLog. Tests would be good too. > > The test got split to patch 11. I don't recall if the test can pass > without patches 9-10 as well. OK. As long as there's a test I'm happy, even if it shows up in a later patch ;-) jeff
On 10/20/2015 03:27 PM, Richard Henderson wrote: > > +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID (addr_space_t @var{as}) > +Define this to modify the default handling of address 0 for the > +address space. Return true if 0 should be considered a valid address. > +@end deftypefn > + I'm confused by this new hook. How does it interact with -fdelete-null-pointer-checks? E.g. nios2-elf defaults flag_delete_null_pointer_checks to 0 precisely because address 0 is legitimate on that target. The avr and cr16 backends simply override flag_delete_null_pointer_checks. Do backends that already frob one thing need to frob the other as well? Are there any changes to the user documentation for -fdelete-null-pointer-checks required? -Sandra
On Thu, Oct 22, 2015 at 5:12 AM, Sandra Loosemore <sandra@codesourcery.com> wrote: > On 10/20/2015 03:27 PM, Richard Henderson wrote: >> >> >> +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID >> (addr_space_t @var{as}) >> +Define this to modify the default handling of address 0 for the >> +address space. Return true if 0 should be considered a valid address. >> +@end deftypefn >> + > > > I'm confused by this new hook. How does it interact with > -fdelete-null-pointer-checks? E.g. nios2-elf defaults > flag_delete_null_pointer_checks to 0 precisely because address 0 is > legitimate on that target. The avr and cr16 backends simply override > flag_delete_null_pointer_checks. Do backends that already frob one thing > need to frob the other as well? Are there any changes to the user > documentation for -fdelete-null-pointer-checks required? I suppose a cleanup possibility would be to get rid of that "abuse" of flag_delete_null_pointer_checks and make all targets now defining that to zero instead implement the hook above but for the default address-space. And then make all places checking for flag_delete_null_pointer_checks (or at least some) use the target hook instead. Richard. > -Sandra >
On 10/21/2015 05:12 PM, Sandra Loosemore wrote: > On 10/20/2015 03:27 PM, Richard Henderson wrote: >> >> +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID >> (addr_space_t @var{as}) >> +Define this to modify the default handling of address 0 for the >> +address space. Return true if 0 should be considered a valid address. >> +@end deftypefn >> + > > I'm confused by this new hook. How does it interact with > -fdelete-null-pointer-checks? It's kind of like -fdelete-null-pointer-checks on a per-address-space basis. > E.g. nios2-elf defaults > flag_delete_null_pointer_checks to 0 precisely because address 0 is > legitimate on that target. The avr and cr16 backends simply override > flag_delete_null_pointer_checks. Do backends that already frob one thing > need to frob the other as well? No. All three of these targets have address 0 valid in the generic address space, so there it might be best to leave things as-is. > Are there any changes to the user documentation for > -fdelete-null-pointer-checks required? I shouldn't think so. r~
On 10/22/2015 01:58 AM, Richard Biener wrote: > On Thu, Oct 22, 2015 at 5:12 AM, Sandra Loosemore > <sandra@codesourcery.com> wrote: >> On 10/20/2015 03:27 PM, Richard Henderson wrote: >>> >>> >>> +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID >>> (addr_space_t @var{as}) >>> +Define this to modify the default handling of address 0 for the >>> +address space. Return true if 0 should be considered a valid address. >>> +@end deftypefn >>> + >> >> >> I'm confused by this new hook. How does it interact with >> -fdelete-null-pointer-checks? E.g. nios2-elf defaults >> flag_delete_null_pointer_checks to 0 precisely because address 0 is >> legitimate on that target. The avr and cr16 backends simply override >> flag_delete_null_pointer_checks. Do backends that already frob one thing >> need to frob the other as well? Are there any changes to the user >> documentation for -fdelete-null-pointer-checks required? > > I suppose a cleanup possibility would be to get rid of that "abuse" of > flag_delete_null_pointer_checks and make all targets now defining that to zero > instead implement the hook above but for the default address-space. > And then make all places checking for flag_delete_null_pointer_checks (or > at least some) use the target hook instead. If it's not possible to integrate the two mechanisms somehow, can we at least improve the documentation of the new hook to better explain how it interacts with flag_delete_null_pointer_checks? E.g. I'm concerned that if you provide a hook implementation that always returns true, it won't really disable all optimizations that depend on the assumption that accesses to address 0 will trap, and that you still need to set flag_delete_null_pointer_checks too. The language I quoted above is pretty vague about what compiler actions are controlled by this flag, and if it only controls a subset of assumptions made about address zero it ought to be more explicit about what subset that is. -Sandra
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index ec19a57..8f833d1 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -53707,6 +53707,16 @@ ix86_addr_space_convert (rtx op, tree from_type, tree to_type) #undef TARGET_ADDR_SPACE_CONVERT #define TARGET_ADDR_SPACE_CONVERT ix86_addr_space_convert +/* All use of segmentation is assumed to make address 0 valid. */ + +static bool +ix86_addr_space_zero_address_valid (addr_space_t as) +{ + return as != ADDR_SPACE_GENERIC; +} +#undef TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID +#define TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID ix86_addr_space_zero_address_valid + /* Initialize the GCC target structure. */ #undef TARGET_RETURN_IN_MEMORY #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 731e630..d530a9c 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -10316,6 +10316,11 @@ arithmetic operations. Pointers to a superset address space can be converted to pointers to a subset address space via explicit casts. @end deftypefn +@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID (addr_space_t @var{as}) +Define this to modify the default handling of address 0 for the +address space. Return true if 0 should be considered a valid address. +@end deftypefn + @deftypefn {Target Hook} rtx TARGET_ADDR_SPACE_CONVERT (rtx @var{op}, tree @var{from_type}, tree @var{to_type}) Define this to convert the pointer expression represented by the RTL @var{op} with type @var{from_type} that points to a named address diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 0b52250d..f8dad76 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -7434,6 +7434,8 @@ c_register_addr_space ("__ea", ADDR_SPACE_EA); @hook TARGET_ADDR_SPACE_SUBSET_P +@hook TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID + @hook TARGET_ADDR_SPACE_CONVERT @node Misc diff --git a/gcc/fold-const.c b/gcc/fold-const.c index de45a2c..eb7edca 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -1566,7 +1566,11 @@ const_unop (enum tree_code code, tree type, tree arg0) return fold_convert_const (code, type, arg0); case ADDR_SPACE_CONVERT_EXPR: - if (integer_zerop (arg0)) + /* If the source address is 0, and the source address space + cannot have a valid object at 0, fold to dest type null. */ + if (integer_zerop (arg0) + && !(targetm.addr_space.zero_address_valid + (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))))) return fold_convert_const (code, type, arg0); break; diff --git a/gcc/gimple.c b/gcc/gimple.c index 5312d6e..6aa3638 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -2631,9 +2631,15 @@ nonfreeing_call_p (gimple *call) static bool check_loadstore (gimple *, tree op, tree, void *data) { - if ((TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF) - && operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0)) - return true; + if (TREE_CODE (op) == MEM_REF || TREE_CODE (op) == TARGET_MEM_REF) + { + /* Some address spaces may legitimately dereference zero. */ + addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (op)); + if (targetm.addr_space.zero_address_valid (as)) + return false; + + return operand_equal_p (TREE_OPERAND (op, 0), (tree)data, 0); + } return false; } diff --git a/gcc/target.def b/gcc/target.def index 694e455..e33c87f 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -3185,6 +3185,15 @@ converted to pointers to a subset address space via explicit casts.", bool, (addr_space_t subset, addr_space_t superset), default_addr_space_subset_p) +/* True if 0 is a valid address in the address space, or false if + 0 is a NULL in the address space. */ +DEFHOOK +(zero_address_valid, + "Define this to modify the default handling of address 0 for the\n\ +address space. Return true if 0 should be considered a valid address.", + bool, (addr_space_t as), + default_addr_space_zero_address_valid) + /* Function to convert an rtl expression from one address space to another. */ DEFHOOK (convert, diff --git a/gcc/targhooks.c b/gcc/targhooks.c index f04964b..d221d20 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1256,6 +1256,15 @@ default_addr_space_subset_p (addr_space_t subset, addr_space_t superset) return (subset == superset); } +/* The default hook for determining if 0 within a named address + space is a valid address. */ + +bool +default_addr_space_zero_address_valid (addr_space_t as ATTRIBUTE_UNUSED) +{ + return false; +} + /* The default hook for TARGET_ADDR_SPACE_CONVERT. This hook should never be called for targets with only a generic address space. */ diff --git a/gcc/targhooks.h b/gcc/targhooks.h index 77c284a..ade3327 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -173,6 +173,7 @@ extern bool default_addr_space_legitimate_address_p (machine_mode, rtx, extern rtx default_addr_space_legitimize_address (rtx, rtx, machine_mode, addr_space_t); extern bool default_addr_space_subset_p (addr_space_t, addr_space_t); +extern bool default_addr_space_zero_address_valid (addr_space_t); extern rtx default_addr_space_convert (rtx, tree, tree); extern unsigned int default_case_values_threshold (void); extern bool default_have_conditional_execution (void);