On 05/08/2017 08:38 PM, Martin Sebor wrote: > On 04/28/2017 12:35 PM, Jeff Law wrote: >> On 04/26/2017 11:05 AM, Martin Sebor wrote: >>> On 04/24/2017 03:35 PM, Jeff Law wrote: >>>> On 04/11/2017 12:57 PM, Martin Sebor wrote: >>>>> In a review of my fix for bug 80364 Jakub pointed out that to >>>>> determine whether an argument to an integer directive is of >>>>> an integer type the gimple-ssa-sprintf pass tests the type code >>>>> for equality to INTEGER_TYPE when it should instead be using >>>>> INTEGRAL_TYPE_P(). This has the effect of the pass being unable >>>>> to use the available range of arguments of enumerated types, >>>>> resulting in both false positives and false negatives, and in >>>>> some cases, in emitting suboptimal code. >>>>> >>>>> The attached patch replaces those tests with INTEGRAL_TYPE_P(). >>>>> >>>>> Since this is not a regression I submit it for GCC 8. >>>> You might consider using POINTER_TYPE_P as well. >>> >>> You mean rather than (TREE_CODE (type) == POINTER_TYPE)? Those >>> I believe are vestiges of the %p handling that was removed sometime >>> last year, and (unless you are recommending I remove them as part >>> of this patch) should probably be removed during the next cleanup.Yes, >>> that can be a follow-up cleanup. >> >> For the future, if you find yourself writing something like >> TREE_CODE (TREE_TYPE (x)) == POINTER_TYPE, you're usually going to be >> better off using POINTER_TYPE_P (TREE_TYPE (x)). That allows the code >> to work with C++ references as well as C pointers. > > I'll keep it in mind, thanks. Should I take this as approval > of the patch as is or are there some changes you'd like me to > make? Yes, Jakub's approval still stands. My comment was more for future reference.
Removal of the dead code WRT %p handling seems wise, but makes the most sense as a follow-up. Sorry for the long delay, jeff
