[PATCH] D64111: Add getMostFrequentByte and use for isBytewiseValue implementation

[JF Bastien via Phabricator via llvm-commits]

jfb added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3182
   if (isa<UndefValue>(V))
-    return UndefInt8;
+    return;
 
----------------
eugenis wrote:
> vitalybuka wrote:
> > jfb wrote:
> > > Should `Undef` count as `Other`? Or have its own count?
> > Undef means we don't care what it's there, so we can fill we any pattern.
> > Other counts bytes which are different from most frequent value and will have to be fixed, e.g. after memset
> Should we count undefs separately in the histogram? It seems like it would not add any complexity or runtime cost, and the caller can just add the two number if that's what they need.
Right, it seems like we might want the following information:

* Value is one of 256 bytes were tracking.
* Value wasn't something we looked into, for X bytes.
* Value was `undef`.
* Value was padding (same as `undef`).

We can figure out some of this, but not all, from the current patch. Is it useful to know the rest? I'm not sure! I haven't inspected all uses to be able to convince myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64111