This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add tests for atomic bittest with register/memory operands
ClosedPublic

Authored by goldstein.w.n on Jan 3 2023, 6:40 PM.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Jan 3 2023, 6:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 6:40 PM
Herald added a subscriber: pengfei. · View Herald Transcript
goldstein.w.n requested review of this revision.Jan 3 2023, 6:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 6:40 PM

This is patch [1/2], it is just tests.

Note: This is a follow up to https://reviews.llvm.org/D140645 and is splitting up the reviews for the tests and code.

Thanks for the patch. I suggest we only test either BMI or BMI2. The difference between BMI and BMI2 is not what we care about in these patches.

goldstein.w.n added a comment.EditedJan 3 2023, 8:22 PM

Thanks for the patch. I suggest we only test either BMI or BMI2. The difference between BMI and BMI2 is not what we care about in these patches.

Do you mean test: [neither, bmi1 + bmi2]
or do you mean test [neither]?

If the former already done:

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=-bmi,-bmi2 < %s | FileCheck %s --check-prefixes=X86-NOBMI2
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=+bmi,+bmi2 < %s | FileCheck %s --check-prefixes=X86-BMI2
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=-bmi,-bmi2 < %s | FileCheck %s --check-prefixes=X64-NOBMI2
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi,+bmi2 < %s | FileCheck %s --check-prefixes=X64-BMI2
...

Thanks for the patch. I suggest we only test either BMI or BMI2. The difference between BMI and BMI2 is not what we care about in these patches.

Do you mean test: [neither, bmi1 + bmi2]
or do you mean test [neither]?

If the former already done:

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=-bmi,-bmi2 < %s | FileCheck %s --check-prefixes=X86-NOBMI2
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=+bmi,+bmi2 < %s | FileCheck %s --check-prefixes=X86-BMI2
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=-bmi,-bmi2 < %s | FileCheck %s --check-prefixes=X64-NOBMI2
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi,+bmi2 < %s | FileCheck %s --check-prefixes=X64-BMI2
...

I mean

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=+bmi2 < %s | FileCheck %s --check-prefixes=X86
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi2 < %s | FileCheck %s --check-prefixes=X64

Or

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu < %s | FileCheck %s --check-prefixes=X86
; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s --check-prefixes=X64

Remove BMI tests

Thanks for the patch. I suggest we only test either BMI or BMI2. The difference between BMI and BMI2 is not what we care about in these patches.

Do you mean test: [neither, bmi1 + bmi2]
or do you mean test [neither]?

If the former already done:

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=-bmi,-bmi2 < %s | FileCheck %s --check-prefixes=X86-NOBMI2
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=+bmi,+bmi2 < %s | FileCheck %s --check-prefixes=X86-BMI2
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=-bmi,-bmi2 < %s | FileCheck %s --check-prefixes=X64-NOBMI2
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi,+bmi2 < %s | FileCheck %s --check-prefixes=X64-BMI2
...

I mean

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu -mattr=+bmi2 < %s | FileCheck %s --check-prefixes=X86
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi2 < %s | FileCheck %s --check-prefixes=X64

Or

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=i686-unknown-linux-gnu < %s | FileCheck %s --check-prefixes=X86
; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s --check-prefixes=X64

Done in V2. (drop the BMI ones).

craig.topper retitled this revision from Add tests for atomic bittest with register/memory operands to [Add tests for atomic bittest with register/memory operands.Jan 4 2023, 4:50 PM
craig.topper retitled this revision from [Add tests for atomic bittest with register/memory operands to [X86] Add tests for atomic bittest with register/memory operands.
pengfei added inline comments.Jan 4 2023, 10:04 PM
llvm/test/CodeGen/X86/atomic-rm-bit-test.ll
7393–7429

I think we never have a chance to optimize 64-bit operations on 32-bits, right? Should we move these 64-bit tests to another file and only leave a single run ; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s

goldstein.w.n added inline comments.Jan 4 2023, 11:11 PM
llvm/test/CodeGen/X86/atomic-rm-bit-test.ll
7393–7429

I think we never have a chance to optimize 64-bit operations on 32-bits, right? Should we move these 64-bit tests to another file and only leave a single run ; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s

We get it something i.e:

define zeroext i16 @atomic_shl1_or_16_const_val(ptr %v) nounwind {
; X86-LABEL: atomic_shl1_or_16_const_val:
; X86:       # %bb.0: # %entry
; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
; X86-NEXT:    xorl %eax, %eax
; X86-NEXT:    lock btsw $4, (%ecx)
; X86-NEXT:    setb %al
; X86-NEXT:    shll $4, %eax
; X86-NEXT:    # kill: def $ax killed $ax killed $eax
; X86-NEXT:    retl
pengfei added inline comments.Jan 4 2023, 11:14 PM
llvm/test/CodeGen/X86/atomic-rm-bit-test.ll
7393–7429

I mean just put these define i64 @...64... into another file and only run for 64-bit.

RKSimon added inline comments.Jan 5 2023, 3:15 AM
llvm/test/CodeGen/X86/atomic-rm-bit-test.ll
7393–7429

Yes you probably want 2 files: atomic-rm-bit-test.ll (non-i64 tests tested on 32/64 triples) and atomic-rm-bit-test-x86_64.ll (i64 tests tested just on 64 triples).

Split the x86 and x64 tests

goldstein.w.n marked 2 inline comments as done.Jan 5 2023, 9:31 AM
pengfei accepted this revision.Jan 5 2023, 6:24 PM

LGTM. Do you want me to commit it for you?

llvm/test/CodeGen/X86/atomic-rm-bit-test-64.ll
3

We don't need prefix for single RUN. I can update it for you.

This revision is now accepted and ready to land.Jan 5 2023, 6:24 PM

Remove unnecessary prefix

LGTM. Do you want me to commit it for you?

Yes, thank you!

Removed prefixes.

goldstein.w.n marked an inline comment as done.Jan 5 2023, 11:17 PM
This revision was landed with ongoing or failed builds.Jan 6 2023, 1:55 AM
This revision was automatically updated to reflect the committed changes.