This is an archive of the discontinued LLVM Phabricator instance.

[llvm-exegesis] Handle CMOV's OPERAND_COND_CODE OperandType
ClosedPublic

Authored by lebedev.ri on Apr 1 2019, 2:04 AM.

Details

Summary

D60041 will regress/break CMOV benchmarking.

This at least allows to not crash on the opcode.
However, this still won't generate all the snippets with all the condcode enumerators.

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Apr 1 2019, 2:04 AM

Thanks Roman. I only have a comment regarding documentation.

tools/llvm-exegesis/lib/Target.h
105 ↗(On Diff #193054)

This would need some documentation, e.g.

// Assigns a random operand of the right type to variable Var. The default implementation only handles generic operand types. The target is responsible for handling any operand starting from OPERAND_FIRST_TARGET.
lebedev.ri added inline comments.Apr 1 2019, 5:04 AM
tools/llvm-exegesis/lib/SnippetGenerator.cpp
49 ↗(On Diff #193054)

Any advice on how to best produce multiple variants covering all the condcodes?
The obvious solution could be to pass BenchmarkCode to Target hook that would
duplicate it for each of the possible condcodes,
and return vector that would get appended to Output.
Is that on the right track? Anything cleaner?

lebedev.ri updated this revision to Diff 193067.Apr 1 2019, 5:10 AM

Add comment.

lebedev.ri marked an inline comment as done.Apr 1 2019, 5:10 AM
courbet added inline comments.Apr 1 2019, 5:21 AM
tools/llvm-exegesis/lib/SnippetGenerator.cpp
49 ↗(On Diff #193054)

I think the original idea was to generate several identical templates and let randomization handle exploration. With what you just implemented when generating several instances of the same template you always get the same cond code because you wrote:

AssignedValue = llvm::MCOperand::createImm(1);

if you did something like:

AssignedValue = llvm::MCOperand::createImm(random_int_in_range(0, 2));

Then half of your generated snippets would go each way.

+ @gchatelet who started[[ https://github.com/llvm-mirror/llvm/commit/b1497a74adb1d3669dbb16a9e02ee5214ea211a6 | implementing support for exploration ]].

lebedev.ri added inline comments.Apr 1 2019, 5:38 AM
tools/llvm-exegesis/lib/SnippetGenerator.cpp
49 ↗(On Diff #193054)

Hmm, yes, that is the least intrusive solution, but that is waay too brittle. :(
Is that really the recommended solution here, given that we know all the affected opcodes and the possible condcodes?

gchatelet accepted this revision.Apr 2 2019, 6:22 AM
gchatelet added inline comments.
tools/llvm-exegesis/lib/SnippetGenerator.cpp
49 ↗(On Diff #193054)

Thx for the fix @lebedev.ri
I'm not sure to understand what you mean by waay too brittle. Keep in mind that we want to explore as much as possible and - for sure - we can be more efficient when we know the semantic of a dimension but we also want to discover new behavior that were not known before.
There's a tension between the two obviously. For now I would just stick to this Patch. I'll work on value exploration as soon as possible - I'm especially interested in floating point values.

This revision is now accepted and ready to land.Apr 2 2019, 6:22 AM

Now that the parent patches has landed, will try to rebase and land tomorrow.

This revision was automatically updated to reflect the committed changes.