Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, Jakub, (I am CCing Joseph and Martin for their inputs on how to _selectively_ generate call to .ACCESS_WITH_SIZE for x->p depending on its context in C FE). > On Jul 17, 2025, at 11:40, Jakub Jelinek wrote: > > So say for > struct S { int s; int *p __attribute__((counted_by (s))); }; > >

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 17, 2025, at 11:40, Jakub Jelinek wrote: > > On Thu, Jul 17, 2025 at 03:26:05PM +, Qing Zhao wrote: >> How about add a new flag to distinguish these two cases, and put it to the >> 3th argument: >> >> ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, >> TYPE_OF_SIZ

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Thu, Jul 17, 2025 at 03:26:05PM +, Qing Zhao wrote: > How about add a new flag to distinguish these two cases, and put it to the > 3th argument: > >ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, > TYPE_OF_SIZE + ACCESS_MODE + IS_POINTER, TYPE_SIZE_UNIT > for element)

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, Jakub, I re-read your other email sent last week (see below) in order to understand the email you sent yesterday. -:) And I think that I fully understand your point this time (hopefully -:), see below: > On Jul 7, 2025, at 08:48, Jakub Jelinek wrote: > > The original use of .ACCESS_WIT

Re: ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 16, 2025, at 17:47, Jakub Jelinek wrote: > > On Wed, Jul 16, 2025 at 09:22:19PM +, Qing Zhao wrote: >> Yes, the above solution could also resolve the undefined behavior issue. We >> can certainly go >> with this approach. > > Another option is to use .ACCESS_WITH_SIZE (with diff

Re: ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Wed, Jul 16, 2025 at 09:22:19PM +, Qing Zhao wrote: > Yes, the above solution could also resolve the undefined behavior issue. We > can certainly go > with this approach. Another option is to use .ACCESS_WITH_SIZE (with different flags compared to the FAM cases) solely on reads from the

Re: ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 16, 2025, at 16:38, Jakub Jelinek wrote: > > On Tue, Jul 15, 2025 at 06:39:42PM +, Qing Zhao wrote: >> I re-implemented the patch based on B to fix PR120929, however, the approach >> B brings undefined behavior into the application. >> >> (Actually, I met this issue in the previ

Re: ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Tue, Jul 15, 2025 at 06:39:42PM +, Qing Zhao wrote: > I re-implemented the patch based on B to fix PR120929, however, the approach > B brings undefined behavior into the application. > > (Actually, I met this issue in the previous implementation but forgot to > documented it. > This iss

ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, I wrote a small writeup to summarize the two approaches to generate .ACCESS_WITH_SIZE for pointers with counted_by, In this writeup, I described the major issue for the approach we agreed on last week to fix PR120929, due to the problem, the previous implementation in the committed (and rev

Re: ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 15, 2025, at 02:32, Richard Biener wrote: > > On Mon, Jul 14, 2025 at 10:58 PM Qing Zhao wrote: >> >> >>> On Jul 7, 2025, at 13:07, Qing Zhao wrote: >>> >>> As I mentioned in the latest email I replied to the thread, the original >>> implementation of the counted_by for pointer w

Re: ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Mon, Jul 14, 2025 at 10:58 PM Qing Zhao wrote: > > > > On Jul 7, 2025, at 13:07, Qing Zhao wrote: > > > > As I mentioned in the latest email I replied to the thread, the original > > implementation of the counted_by for pointer was implemented without the > > additional indirection. > > But

ACCESS_WITH_SIZE for pointers Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 7, 2025, at 13:07, Qing Zhao wrote: > > As I mentioned in the latest email I replied to the thread, the original > implementation of the counted_by for pointer was implemented without the > additional indirection. > But that implementation has a fundamental bug during testing. then

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-08 18:18, Qing Zhao wrote: On Jul 8, 2025, at 17:46, Siddhesh Poyarekar wrote: On 2025-07-08 17:17, Qing Zhao wrote: 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? I don't know

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 8, 2025, at 17:46, Siddhesh Poyarekar wrote: > > On 2025-07-08 17:17, Qing Zhao wrote: >> 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? > > I don't know about PR121000, but f

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-08 17:17, Qing Zhao wrote: 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? I don't know about PR121000, but for counted_by with pointers, I think the REF_TO_OBJ (and the result_typ

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, Jakub, Thanks a lot for your comments and suggestions. Please see my questions below: > On Jul 7, 2025, at 17:47, Jakub Jelinek wrote: > > On Mon, Jul 07, 2025 at 09:18:53PM +, Qing Zhao wrote: >> From OLD: >> >> _2 = &a->c; >> _3 = &a->count; >> _1 = .ACCESS_WITH_SIZE (_2, _3, 1, 0, -

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

I just updated PR121000: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121000. Yes, the root cause is exactly what you mentioned in the other email: “ The IL must clearly use the value (the size of the element), otherwise DCE or other passes will happily optimize it away, they don't keep some expr

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, > On Jul 8, 2025, at 01:18, Jakub Jelinek wrote: >> >>>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 >>>

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Tue, Jul 08, 2025 at 08:15:30AM +0200, Richard Biener wrote: > > No. Because (most of the) pointer conversions are useless, if you have > > _2 = &something.fam[0]; > > ... > > _4 = &something.fam; > > _5 = .ACCESS_WITH_SIZE (_4, ...); > > (or whatever else where _4 has the carefully chosen p

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> Am 08.07.2025 um 07:21 schrieb Jakub Jelinek : > > On Mon, Jul 07, 2025 at 07:51:52PM -0400, Siddhesh Poyarekar wrote: >>> On 2025-07-07 17:47, Jakub Jelinek wrote: >>> Even 6 arguments is IMHO too much. >>> /* Expand the IFN_ACCESS_WITH_SIZE function: >>>ACCESS_WITH_SIZE (REF_TO_OBJ, RE

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Mon, Jul 07, 2025 at 07:51:52PM -0400, Siddhesh Poyarekar wrote: > On 2025-07-07 17:47, Jakub Jelinek wrote: > > 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

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-07 17:47, Jakub Jelinek wrote: 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;

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Mon, Jul 07, 2025 at 09:18:53PM +, 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 (_

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, thanks a lot for all the discussion so far on this issue. An update on this: 1. I have reverted the 3 patches to support counted_by for pointers I have committed last week from master. 2. At the same time: On the C FE code generation to .ACCESS_WITH_SIZE for pointers with counted_by att

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-07 13:07, Qing Zhao wrote: If the current bug is urgent to be fixed. and you are not comfortable with the simple Patch Sid provided, then I am okay to back it out now and then push it back with the fix to this current bug at a later time after everyone is comfortable with the curren

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 7, 2025, at 11:58, Siddhesh Poyarekar wrote: > > On 2025-07-07 09:33, Siddhesh Poyarekar wrote: >>> The only difference between &a->fam[0] and &a->fam is not the value (that is >>> the same), just the type in one case say int *, in the other int [0:] *. >>> At least in GIMPLE pointer

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> On Jul 7, 2025, at 02:05, Richard Biener wrote: > > On Sat, Jul 5, 2025 at 2:10 PM Siddhesh Poyarekar wrote: >> >> On 2025-07-05 07:23, Richard Biener wrote: OK, should I revert right away or can we wait till Qing returns on Monday? >>> >>> Monday is OK with me. >>> >> >> Thanks, so

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-07 09:33, Siddhesh Poyarekar wrote: The only difference between &a->fam[0] and &a->fam is not the value (that is the same), just the type in one case say int *, in the other int [0:] *. At least in GIMPLE pointer conversions are useless, so what exact type of the argument is doesn't m

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

Hi, Sorry for the late reply. And thanks a lot for all the help so far. For the documantion of the .ACCESS_WITH_SIZE for pointers, please see the following in c/c-typeck.cc : /* Given a COMPONENT_REF REF with the location LOC, the corresponding COUNTED_BY_REF, and the

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-07 09:14, Jakub Jelinek wrote: So that ought to be &a->fam[0] right, and not &a->fam? It means the same for a FAM, so why not specify it as &a->fam[0] (or simply a->fam)? That will be consistent with when fam is a pointer. The only difference between &a->fam[0] and &a->fam is not th

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Mon, Jul 07, 2025 at 09:07:14AM -0400, Siddhesh Poyarekar wrote: > On 2025-07-07 08:48, Jakub Jelinek wrote: > > > The return value of .ACCESS_WITH_SIZE clobbering PTR (that subsequently > > > gets > > > passed to __builtin_dynamic_object_size) should be sufficient to fully > > > prevent the re

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-07 08:48, Jakub Jelinek wrote: The return value of .ACCESS_WITH_SIZE clobbering PTR (that subsequently gets passed to __builtin_dynamic_object_size) should be sufficient to fully prevent the reordering, it shouldn't have to clobber &PTR, I think. The original use of .ACCESS_WITH_SIZE

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Mon, Jul 07, 2025 at 07:21:26AM -0400, Siddhesh Poyarekar wrote: > > is .ACCESS_WITH_SIZE documented? I can't find it documented in the > > internals manual, internal-fn.def has > > It's documented in tree-object-size.cc as: > > /* Compute __builtin_object_size for a CALL to .ACCESS_WITH_SIZE

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-07 03:05, Richard Biener wrote: Since the intent of the .ACCESS_WITH_SIZE was to associate the storage of count with c to prevent reordering, maybe the semantically correct solution here is that when c is a pointer, the frontend emits: _2 = a->c; _3 = &a->count; _1 = .ACCE

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On Sat, Jul 5, 2025 at 2:10 PM Siddhesh Poyarekar wrote: > > On 2025-07-05 07:23, Richard Biener wrote: > >> OK, should I revert right away or can we wait till Qing returns on Monday? > > > > Monday is OK with me. > > > > Thanks, so I thought about this some more and I think when I said in > bugzi

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-05 07:23, Richard Biener wrote: OK, should I revert right away or can we wait till Qing returns on Monday? Monday is OK with me. Thanks, so I thought about this some more and I think when I said in bugzilla: "In fact, maybe the .ACCESS_WITH_SIZE handling in objsz probably needs

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> Am 05.07.2025 um 12:19 schrieb Siddhesh Poyarekar : > > On 2025-07-05 02:45, Richard Biener wrote: Am 04.07.2025 um 19:57 schrieb Siddhesh Poyarekar : >>> >>> On 2025-07-04 08:12, Siddhesh Poyarekar wrote: > On 2025-07-04 08:08, Siddhesh Poyarekar wrote: > gcc/ChangeLog: >

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-05 02:45, Richard Biener wrote: Am 04.07.2025 um 19:57 schrieb Siddhesh Poyarekar : On 2025-07-04 08:12, Siddhesh Poyarekar wrote: On 2025-07-04 08:08, Siddhesh Poyarekar wrote: gcc/ChangeLog: I forgot to add the PR number to the ChangeLog entries, I've fixed it in my commit m

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

> Am 04.07.2025 um 19:57 schrieb Siddhesh Poyarekar : > > On 2025-07-04 08:12, Siddhesh Poyarekar wrote: >>> On 2025-07-04 08:08, Siddhesh Poyarekar wrote: >>> gcc/ChangeLog: >>> >> I forgot to add the PR number to the ChangeLog entries, I've fixed it in my >> commit message. >>> * tree-

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-04 08:12, Siddhesh Poyarekar wrote: On 2025-07-04 08:08, Siddhesh Poyarekar wrote: gcc/ChangeLog: I forgot to add the PR number to the ChangeLog entries, I've fixed it in my commit message. * tree-object-size.cc (is_access_with_size): New function. (collect_object_sizes

Re: [PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

On 2025-07-04 08:08, Siddhesh Poyarekar wrote: gcc/ChangeLog: I forgot to add the PR number to the ChangeLog entries, I've fixed it in my commit message. * tree-object-size.cc (is_access_with_size): New function. (collect_object_sizes_for): Use it. gcc/testsuite/ChangeLog:

[PATCH] tree-optimization/120929: Limit MEM_REF handling to .ACCESS_WITH_SIZE

r16-1905-g7165ca43caf470 incorrectly returned the size of *_1 for a GIMPLE_ASSIGN of type: ptr = *_1; This is only OK when _1 is set to .ACCESS_WITH_SIZE, since that builtin expresses the size of *_1 in the form of _1. gcc/ChangeLog: * tree-object-size.cc (is_access_with_size): New fu