Page MenuHomePhabricator

[gicombiner] Add the run-time rule disable option
ClosedPublic

Authored by dsanders on Oct 3 2019, 6:02 PM.

Details

Summary

Each generated helper can be configured to generate an option that disables
rules in that helper. This can be used to bisect rulesets.

The disable bits are stored in a SparseVector as this is very cheap for the
common case where nothing is disabled. It gets more expensive the more rules
are disabled but you're generally doing that for debug purposes where
performance is less of a concern.

Depends on D68426

Diff Detail

Event Timeline

dsanders created this revision.Oct 3 2019, 6:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2019, 6:02 PM
volkan added inline comments.Oct 15 2019, 2:06 PM
llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
3

No need to pass -O0 and -global-isel as it runs a specific pass.

6

Could you add a test case that disables the rule using the ID?

llvm/utils/TableGen/GICombinerEmitter.cpp
401

Looks like it's not possible to disable a rule in Release mode. I thought it'd support disabling by rule id, could you double check?

dsanders updated this revision to Diff 225131.Oct 15 2019, 4:28 PM
dsanders marked 4 inline comments as done.
  • Fix up nits
llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-copy-prop-disabled.mir
6

It's not very practical to do so. The ID is stable for a given revision of the compiler but isn't guaranteed to be stable as more rules are added.

llvm/utils/TableGen/GICombinerEmitter.cpp
401

Oops. That's from before I changed my mind about supporting it in Release mode. Fixed it

dsanders updated this revision to Diff 225134.Oct 15 2019, 4:47 PM
  • setRuleDisabled wasn't quite right w.r.t disabling by id either
volkan accepted this revision.Oct 15 2019, 5:50 PM

LGTM.

This revision is now accepted and ready to land.Oct 15 2019, 5:50 PM
This revision was automatically updated to reflect the committed changes.
sdmitriev added inline comments.
llvm/utils/TableGen/GICombinerEmitter.cpp
372

getRuleIdxForIdentifier() is defined under #ifndef NDEBUG, but called without #ifndef NDEBUG. I assume this will cause an error if NDEBUG macro is defined.

dsanders marked an inline comment as done.Oct 17 2019, 10:19 AM
dsanders added inline comments.
llvm/utils/TableGen/GICombinerEmitter.cpp
372

Yes, you're right. The sanitizer-x86_64-linux-fuzzer bot complained at me about that and I fixed it in r375067

Thanks