Hi, Jakub, Thanks a lot for your comments and suggestions. Please see my questions below:
> On Jul 7, 2025, at 17:47, Jakub Jelinek <ja...@redhat.com> wrote: > > On Mon, Jul 07, 2025 at 09:18:53PM +0000, Qing Zhao wrote: >> From OLD: >> >> _2 = &a->c; >> _3 = &a->count; >> _1 = .ACCESS_WITH_SIZE (_2, _3, 1, 0, -1, 0B); >> _4 = *_1; >> D.2964 = __builtin_dynamic_object_size (_4, 1); >> >> To NEW: >> >> _2 = a->c; >> _3 = &a->count; >> _1 = .ACCESS_WITH_SIZE (_2, _3, 1, 0, -1, 0B, 0); >> D.2964 = __builtin_dynamic_object_size (_, 1); >> >> >> NOTE, in the above, in addition to pass “a->c” instead of “&a->c” as the >> first parameter, I also >> added one more argument for .ACCESS_WITH_SIZE: >> >> + the 7th argument of the call is 1 when for FAM, 0 for pointers. >> >> To distinguish whether this .ACCESS_WITH_SIZE is for FAM or for pointers. >> And this argument will be used in tree-object-size.cc >> <http://tree-object-size.cc/> to get the element_type of the associated FAM >> or pointer array. > > Even 6 arguments is IMHO too much. > /* Expand the IFN_ACCESS_WITH_SIZE function: > ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, > TYPE_OF_SIZE, ACCESS_MODE) > which returns the REF_TO_OBJ same as the 1st argument; > > 1st argument REF_TO_OBJ: The reference to the object; > 2nd argument REF_TO_SIZE: The reference to the size of the object, > 3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE > represents > 0: the number of bytes. > 1: the number of the elements of the object type; > 4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the > TYPE > of the object referenced by REF_TO_SIZE > 5th argument ACCESS_MODE: > -1: Unknown access semantics > 0: none > 1: read_only > 2: write_only > 3: read_write > 6th argument: A constant 0 with the pointer TYPE to the original flexible > array type. > > I agree with argument 1 and 2 and agree we need 2 INTEGER_CST arguments with > the 2 pointer types. Nobody says those 2 arguments have to be 0 though, > they can be some other INTEGER_CST, similarly how MEM_REF's second argument > is INTEGER_CST with type meaning something and value something different. > Perhaps one can be that -1/0/1/2/3 and another one a bitmask for the > remaining flags, or one can be say 0/1/2/3/4 ored with 0/8 ored with 0/16. > > Though, it is unclear to me how the "the number of the elements of the > object type" actually works. If the FAM has constant sized elements > or pointer points to constant sized element, I agree you can just grab the > size from TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (gimple_call_arg (call, 5)))) > But what if the FAM has a variable length type or it is pointer to VLA? > Trying to use TYPE_SIZE_UNIT will not really work well in that case, while > perhaps during gimplification it will be gimplified and exist, later > optimizations will not see it being used and can optimize it away. > If all you care is to get the size from that, why don't you just pass > the size as the argument? So instead of that 0: the number of bytes > 1: the number of the elements of the object type + the former 6th > argument just pass one argument, 1 if it is the "the number of bytes" > case and some other number, the size of the element. So in all cases > the size in bytes is effectively *(type_of_size *)ref_to_size * eltsz > This argument would be INTEGER_CST whenever it is not VLA or the VLA size. From my understanding of the above comments, there are mainly two problems in the current design of .ACCESS_WITH_SIZE: 1. The correctness issue: As shown in PR121000: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121000: The size of the element of the FAM actually _cannot_ be reliably gotten from TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (gimple_call_arg (call, 5)))) When the element of the FAM has a variable length type. In order to resolve this correctness issue, in addition to the current information that we pass to the call to .ACCESS_WITH_SIZE, we SHOULD pass an additional information, i.e. the original TYPE_SIZE_UNIT of the element TYPE of the FAM to the call to .ACCESS_WITH_SIZE. And with this additional information, we don’t need to distinguish whether the .ACCESS_WITH_SIZE is for FAMs or for pointers anymore. As a result, the new ACCESS_WITH_SIZE is: (change the 6th argument to the TYPE_SIZE_UNIT of the element TYPE of the FAM or the pointer points to) "ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, TYPE_OF_SIZE, ACCESS_MODE) which returns the REF_TO_OBJ same as the 1st argument; 1st argument REF_TO_OBJ: The reference to the object; 2nd argument REF_TO_SIZE: The reference to the size of the object, 3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents 0: the number of bytes. 1: the number of the elements of the object type; 4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE of the object referenced by REF_TO_SIZE 5th argument ACCESS_MODE: -1: Unknown access semantics 0: none 1: read_only 2: write_only 3: read_write 6th argument: The TYPE_SIZE_UNIT of the element TYPE of the FAM or the pointer array. " Are all the above information necessary, minimum and enough? 2. The memory efficiency issue with all the arguments of the .ACCESS_WITH_SIZE. from my understanding, you suggested to combine the argument "constant 0 with TYPE” and the flag arguments to efficiently use the TYPE and the VALUE for different purposes, for example, the combine the above 3th and 4th as one argument, i.e: A constant with its TYPE being the same as the TYPE of the object referenced by REF_TO_SIZE; its value represents CLASS_OF_SIZE: 0: the number of bytes. 1: the number of the elements of the object type; Since after the above 1, we only have 1 constant 0 with TYPE argument left, so, we can only do the above combination. With this improvement, the new ACCESS_WITH_SIZE is: "ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, TYPE_OF_SIZE, ACCESS_MODE) which returns the REF_TO_OBJ same as the 1st argument; 1st argument REF_TO_OBJ: The reference to the object; 2nd argument REF_TO_SIZE: The reference to the size of the object, 3rd argument CLASS_OF_SIZE + TYPE_OF_SIZE: An integer constant with a TYPE: The integer constant value of this argument represents: 0: means that the size referenced by the REF_TO_SIZE is the number of bytes. 1: means that the size referenced by the REF_TO_SIZE is the number of the elements of the object type; The TYPE is the same as the TYPE of the object referenced by REF_TO_SIZE. 4th argument ACCESS_MODE: -1: Unknown access semantics 0: none 1: read_only 2: write_only 3: read_write 5th argument: The TYPE_SIZE_UNIT of the element TYPE of the FAM or the pointer array. “ Are the above the correct and efficient updates to the .ACCESS_WITH_SIZE to resolve both PR121000 and the issue we have with counted_by for pointers? Let me know if you have any more comments or suggestions? Thanks. Qing > Jakub >
