[PATCH] D85012: [NOT FOR COMMIT(yet)] [deadargelim] Use entry values for unused args

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:1843-1853
+    // We might already have generated the entry values within deadargelim,
+    // so we don't consider these as candidates for entry values anymore.
+    if (MI.isDebugValue() && !MI.isDebugEntryValue())
+      recordEntryValue(MI, DefinedRegs, OpenRanges, VarLocIDs);
   }
 
   // Initialize per-block structures and scan for fragment overlaps.
----------------
dblaikie wrote:
> Hmm - how do we use entry values, if we don't know what parts of the parameter's range are valid? Does this only recover entry values for descriptions that are about to get clobbered here? Not for descriptions that became dead/were removed in previous optimization passes?
> 
> (yeah, guess that means we're missing a lot of entry value use, eg:
> ```
> void f1(int);
> void f2(int i) {
>   i = i + 5; // line 3
>   f1(3);
> }
> ```
> We get an entry value if line 3 is deleted, but otherwise we don't get one/only get a location that's valid up until the 'f1' call.
> Similarly if line 3 was `int j = i + 5;` an entry_value location could be used to describe that but isn't. Oh, even if "int j = i;" is used, that doesn't get an entry value location.
> 
> I guess overall my point would be we should be creating entry_values a lot more in intermediate passes. 
> 
> Hmm, I wonder if we'd ever need to combine these strategies - but I guess not. If a location is already described with an entry value, no need to use another. Oh, if we do end up with the work to describe a variable's value with multiple IR Registers - then we might want this. ALso if we are able to use entry_value not at the top-level, then maybe this patch is incorrect? (maybe leave a FIXME about generalizing this for those cases? Could write IR test cases, well at least for the non-top-level: an entry_value + 5, for instance?kEven if no pass is creating such a thing right now, it's probably good/reasonable to support it at the IR level?)
Actually, this piece of code here (on the line 1852) is unwanted (maybe), I need to test it.

In general, entry_value depends on call-site info (call_value attribute of corresponding call-site-param). We generate entry_values in the callee as a backup locations, and we don't know whether it will turn into real value. The more call-site-parameters the more of these will be real values.
Currently, we  generate entry_values *only* for unmodified parameters. We can use entry_value op for modified params as well, if the modification could be expressed in terms of its entry value. There are TODOs within `isEntryValueCandidate()`. I remember I tried to add support for that case here (D68973), but it was quick-made-patch, and I've never returned to it. My opinion is that such support would be easier for the implementation on the IR level, and we might want to implement it there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85012/new/

https://reviews.llvm.org/D85012