Page MenuHomePhabricator

[X86] Use inlineasm flag output for the _bittest* intrinsics.
ClosedPublic

Authored by craig.topper on Sep 18 2020, 1:13 AM.

Details

Summary

Instead of expliciting emitting a setc in the inline asm instructions,
we can use flag output. This allows the backend to use the flag
directly if it is needed by a branch. Previously we needed a test
instruction to convert the register back to a flag.

If the flag can't be used directly, the backend will emit a setcc.

Diff Detail

Event Timeline

craig.topper created this revision.Sep 18 2020, 1:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 18 2020, 1:13 AM
craig.topper requested review of this revision.Sep 18 2020, 1:13 AM

Do we have sufficient backend test coverage for this?

Do we have sufficient backend test coverage for this?

Taking a closer look, I don't think our coverage is very good. We just have one test for each the 28 condition code strings we support and they are all identical tests that just change the string. So we don't have tests that use the condition code as a branch condition. But the code just emits a EFLAGS copy and X86ISD::SETCC so existing combines probably just work.

I did fine a copy paste mistake on one of the condition strings while reviewing the asm flags code. Fixed in 58ecbbcdcddf80b289c449b86d5996d58fba0d78

craig.topper planned changes to this revision.Sep 18 2020, 10:34 PM

@rnk should this inline asm have a memory clobber on it?

rnk added a comment.Sep 21 2020, 11:56 AM

Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them.

In D87888#2285878, @rnk wrote:

Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them.

The middle end may not care about the memory clobber. I think in the backend the memory clobber will cause mayLoad and mayStore to be set on the INLINASM instruction. Not sure if that does anything that sideeffects didn't already do.

Rebase and add a backend test file

Herald added a project: Restricted Project. · View Herald TranscriptSun, Sep 27, 8:38 PM
rnk accepted this revision.Mon, Sep 28, 11:36 AM

Yep, neat.

This revision is now accepted and ready to land.Mon, Sep 28, 11:36 AM