Page MenuHomePhabricator

[mlir] Set random starting index
Needs ReviewPublic

Authored by jpienaar on Nov 1 2020, 2:00 PM.

Details

Reviewers
rriddle
Summary

Example prodding the start of the printed ID sequence to identify cases where a
test is depending on current numbering starting at 0. Might be useful to flag
tests too constrained that will cause pain when updating.

Using MINSTD and LINE as simple (and might even be overkill as 5 as
starting value would work too).

More of an exploration than something we have to do :)

Diff Detail

Event Timeline

jpienaar created this revision.Nov 1 2020, 2:00 PM
Herald added a project: Restricted Project. · View Herald Transcript
jpienaar requested review of this revision.Nov 1 2020, 2:00 PM
rdzhabarov added inline comments.Nov 2 2020, 9:34 AM
mlir/lib/IR/AsmPrinter.cpp
559

qq: do we document anywhere these keys used for tweaking different things?

rriddle added inline comments.Nov 2 2020, 10:15 AM
mlir/lib/IR/AsmPrinter.cpp
559

I think we should align under fewer macros, if this is something that we actually want to do. Seems like randomized naming starts, could be something under EXPENSIVE_CHECKS.

568

Is this guaranteed not to produce duplicates?

rdzhabarov added inline comments.Nov 2 2020, 10:44 AM
mlir/lib/IR/AsmPrinter.cpp
559

+1 to River's point.

568

I was playing with different seeds and seems like i have not found collisions after 10M+ iterations.
Curious, if there is math supporting this for any seeds we could use potentially use?

(also for our purposes, we could fix seed to predefined value, this will guarantee non duplicates for large number of iterations and also would work for our testing purposes).

mehdi_amini added inline comments.Nov 2 2020, 1:58 PM
mlir/lib/IR/AsmPrinter.cpp
559

This isn't an "expensive" check though. Seems closer to the LLVM_ENABLE_REVERSE_ITERATION macro in the mindset.

jpienaar marked 2 inline comments as done.Nov 2 2020, 3:24 PM
jpienaar added inline comments.
mlir/lib/IR/AsmPrinter.cpp
559

Correct, this isn't too expensive nor a would fit in with the other expensive checks. It just makes debug string printing more expensive.

Re: documentation, this was currently just for discussion so I didn't add any and we don't have anything this invasive with ifdefs at the moment (NDEBUG is almost as much as we have I think). If folks think useful to flush out bad tests, we could document in a separate md file. Depends on how many of these we add, we could either also document in cmake or use a config file.

568

Re: duplicates, not until it has gone full cycle back to the start (once we hit 2^31 value IDs in a module). Seed of 0 doesn't work, beyond that any 32-bit seed should work I believe.

This is:

Park, Stephen K.; Miller, Keith W. (1988). "Random Number Generators: Good Ones Are Hard To Find" (PDF). Communications of the ACM. 31 (10): 1192–1201. doi:10.1145/63039.63042.