> > Notice the ??? comment. The code does not set clobbers here because it > > assumes that tree-ssa-alias will do the right thing. > > So one may make builtins handling first, PTA next and only if both say > > "may alias" continue. Other option is to extend the code here to add > > propert clobber/use PTA to make things more regular. I would be happy > > to help with that. > > Or avoid setting the gimple_call_clobber/use sets for builtins handled in > the above way. As said, the info there is currently incorrect. Note that > originally the call use/clobber vi were only added in IPA mode. I suppose > you now add them unconditionally (I forgot about all the changes).
I think even before my changes we did non-trivial call clobbers in some cases. I just generalized existing clode that was generating non-trivial constraints, for example, for readonly non-escape args. If you search in gcc10 tree, call clobber vi is set in handle_rhs_call and some builtins handling and later copied to the gimple_call_clobber in compute_points_to_sets which is the non-ipa path. Similarly there is logic for call use for pure/const etc. At some point of fnspec development I disabled the special case path in tree-ssa-alias.c and it also broke on some builtins testcases due to wrong PTA and this happened before the PTA changes. That is how I know about the ??? comment. With my changes I think it only started to hit more cases mostly because I added the global memory handling which makes the clobber/use sets more likely to disambiguate. I agree that we want to get rid of wrong uses/clobbers from the IL - it is slopy to rely that other oracle will prevail PTA's wrong answers. If we want to just keep clobber/uses set to "all" I guess we need some special way to communicate that given call is specially handled builtin (listing all builtins for third time there seems like to too cool option) or we want to initialize call_use_vi/call_clobber_vi in find_func_aliases_for_builtin_call and in that case maybe it is not that hard to initialize them to the right thing. Do we know what makes ref_clobbered_by_call slow on optabs? (We have snow so I would like to spend some time on cross country ski today and tomorrow, but can definitly profile that at thursday). Honza > > Richard. > > > > > Honza > > > > > > > > It looks like Honza added computation of these sets at some point, also > > > > maybe > > > > forgetting to set some bits here. > > > > > > > > I prefer to investigate / fix this before refactoring the uses in this > > > > way. Can > > > > you file a bugreport please? I can reproduce the memcpy FAIL with > > > > just moving check_fnspec down. > > > > > > So the issue seems that for > > > # .MEM_22 = VDEF <.MEM_19> > > > memcpy (&dst, &src, 1024); > > > > > > determine_global_memory_access determines the stmt doesn't write > > > to global memory (well, yes!), but it fails to add the actual arguments > > > to the set of clobbered vars. > > > > > > Honza, can you please look into this? Seems to be caused > > > by g:008e7397dad971c03c08fc1b0a4a98fddccaaed8 > > > > > > Richard. > > > > > > > Thanks, > > > > Richard. > > > > > > > > > Setting ASSERTS to 0 gives: > > > > > > > > > > FAIL: gcc.c-torture/execute/memcpy-1.c -O2 execution test > > > > > FAIL: gcc.c-torture/execute/memcpy-1.c -O2 -flto > > > > > -fno-use-linker-plugin -flto-partition=none execution test > > > > > FAIL: gcc.c-torture/execute/memcpy-1.c -O2 -flto > > > > > -fuse-linker-plugin -fno-fat-lto-objects execution test > > > > > FAIL: gcc.c-torture/execute/memcpy-1.c -O3 -fomit-frame-pointer > > > > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution > > > > > test > > > > > FAIL: gcc.c-torture/execute/memcpy-1.c -O3 -g execution test > > > > > FAIL: gcc.c-torture/execute/memcpy-1.c -Os execution test > > > > > FAIL: gcc.c-torture/execute/memset-3.c -O3 -fomit-frame-pointer > > > > > -funroll-loops -fpeel-loops -ftracer -finline-functions execution > > > > > test > > > > > FAIL: gcc.c-torture/execute/memset-3.c -O3 -g execution test > > > > > > > > > > # of expected passes 23138 > > > > > # of unexpected failures 8 > > > > > # of unsupported tests 24 > > > > > > > > > > (same for -m32). > > > > > > > > > > Originally I saw it as a bootstrap failure building stage 2 > > > > > build-side libcpp, due to a bogus uninitialised warning. > > > > > > > > > > But alongside the memset and free examples, there's: > > > > > > > > > > /* All the following functions do not return pointers, do not > > > > > modify the points-to sets of memory reachable from their > > > > > arguments and do not add to the ESCAPED solution. */ > > > > > case BUILT_IN_SINCOS: > > > > > case BUILT_IN_SINCOSF: > > > > > ... > > > > > return true; > > > > > > > > > > (always an empty set), whereas check_fnspec takes errno into account. > > > > > > > > > > > I wonder if, at this point, we should split the patch up to do the > > > > > > trivial move of the ao_ref_base dependent and volatile checks > > > > > > and leave this more complicated detail for a second patch? > > > > > > > > > > Yeah, that'd work, but in practice it was the PT part that was the > > > > > high-value part. IIRC the early base checks caught less than 1% of > > > > > cases (I can rerun tonight to get exact numbers). > > > > > > > > > > Thanks, > > > > > Richard > > > > >
