This is an archive of the discontinued LLVM Phabricator instance.

[X86][2/2] Adding atomic add/sub/or/xor/and lowering into RAO-INT instructions
Changes PlannedPublic

Authored by pengfei on Oct 15 2022, 8:05 PM.

Diff Detail

Event Timeline

pengfei created this revision.Oct 15 2022, 8:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2022, 8:05 PM
pengfei requested review of this revision.Oct 15 2022, 8:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2022, 8:05 PM
pengfei updated this revision to Diff 468050.Oct 15 2022, 8:12 PM

Fix comment typo.

It's not immediately clear to me that these are intended as a general atomic replacement. Is there more detail documentation or usage guide anywhere?

craig.topper added inline comments.Oct 15 2022, 8:42 PM
llvm/lib/Target/X86/X86ISelLowering.cpp
31802

aligned*

efriedma requested changes to this revision.Oct 15 2022, 8:47 PM
efriedma added a subscriber: efriedma.

I'd rather not do this until we have:

  1. Some published document that describes the relationship of these new instructions with the C++ atomic model, including interactions with existing atomic lowering. (I don't trust that it's correct based on the two sentences in the manual.)
  2. Some idea whether this is actually preferable; mfence is expensive.
This revision now requires changes to proceed.Oct 15 2022, 8:47 PM

It's not immediately clear to me that these are intended as a general atomic replacement. Is there more detail documentation or usage guide anywhere?

Not yet other than the ISE. I read several documents about the general atomic, but the RAO-INT is a new technology. I'm not very sure the relationships between them.

I'd rather not do this until we have:

  1. Some published document that describes the relationship of these new instructions with the C++ atomic model, including interactions with existing atomic lowering. (I don't trust that it's correct based on the two sentences in the manual.)
  2. Some idea whether this is actually preferable; mfence is expensive.

Yeah, good points, thanks @efriedma. I use mfence for conservative reasons. I had the impression mfence is cheaper than lock. Maybe I was wrong? Anyway, I can make it only work for monotonic. This should be enough as a beginning.

pengfei updated this revision to Diff 468051.Oct 15 2022, 9:17 PM
pengfei marked an inline comment as done.

Address review comments.

pengfei planned changes to this revision.Oct 18 2022, 8:24 AM
Matt added a subscriber: Matt.Oct 19 2022, 5:14 PM