[X86]Add support for outbyte/word/dword and inbyte/word/dword #93774

MalaySanghi · 2024-05-30T06:41:13Z

A previous commit implemented _inp/w/d and renaming that to __inbyte/word/dword

These intrinsics were declared in the header but never implemented.

llvmbot · 2024-05-30T06:41:46Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Malay Sanghi (MalaySanghi)

Changes

Supported: outp, outpw, _outp, _outpw, _outpd
These functions were removed from the Windows runtime library, but for kernel mode development it's still supported

Full diff: https://github.com/llvm/llvm-project/pull/93774.diff

2 Files Affected:

(modified) clang/lib/Headers/intrin.h (+20)
(modified) clang/test/CodeGen/X86/ms-x86-intrinsics.c (+85)

diff --git a/clang/lib/Headers/intrin.h b/clang/lib/Headers/intrin.h
index 5ceb986a1f652..21a3f030216e6 100644
--- a/clang/lib/Headers/intrin.h
+++ b/clang/lib/Headers/intrin.h
@@ -329,6 +329,26 @@ static __inline__ void __DEFAULT_FN_ATTRS __stosq(unsigned __int64 *__dst,
 static __inline__ void __DEFAULT_FN_ATTRS __halt(void) {
   __asm__ volatile("hlt");
 }
+
+static __inline__ int __DEFAULT_FN_ATTRS _outp(unsigned short port, int data) {
+  __asm__ volatile("outb %b0, %w1" : : "a"(data), "Nd"(port) : "memory");
+  return data;
+}
+
+static __inline__ unsigned short __DEFAULT_FN_ATTRS
+_outpw(unsigned short port, unsigned short data) {
+  __asm__ volatile("outw %w0, %w1" : : "a"(data), "Nd"(port) : "memory");
+  return data;
+}
+
+static __inline__ unsigned long __DEFAULT_FN_ATTRS _outpd(unsigned short port,
+                                                          unsigned long data) {
+  __asm__ volatile("outl %k0, %w1" : : "a"(data), "Nd"(port) : "memory");
+  return data;
+}
+
+#define outp(port, data) _outp(port, data)
+#define outpw(R, D) _outpw(port, data)
 #endif
 
 #if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
diff --git a/clang/test/CodeGen/X86/ms-x86-intrinsics.c b/clang/test/CodeGen/X86/ms-x86-intrinsics.c
index aa557c8e19a83..e990ccd3449ac 100644
--- a/clang/test/CodeGen/X86/ms-x86-intrinsics.c
+++ b/clang/test/CodeGen/X86/ms-x86-intrinsics.c
@@ -63,6 +63,91 @@ unsigned __int64 test__emulu(unsigned int a, unsigned int b) {
 // CHECK: [[RES:%[0-9]+]] = mul nuw i64 [[Y]], [[X]]
 // CHECK: ret i64 [[RES]]
 
+//
+// CHECK-I386-LABEL: define dso_local noundef i32 @test_outp(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR3:[0-9]+]], !srcloc [[META4:![0-9]+]]
+// CHECK-I386-NEXT:    ret i32 [[DATA]]
+//
+// CHECK-X64-LABEL: define dso_local noundef i32 @test_outp(
+// CHECK-X64-SAME: i16 noundef [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// CHECK-X64-NEXT:  [[ENTRY:.*:]]
+// CHECK-X64-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR5:[0-9]+]], !srcloc [[META3:![0-9]+]]
+// CHECK-X64-NEXT:    ret i32 [[DATA]]
+//
+int test_outp(unsigned short port, int data) {
+    return _outp(port, data);
+}
+
+//
+// CHECK-I386-LABEL: define dso_local noundef zeroext i16 @test_outpw(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i16 noundef returned zeroext [[DATA:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outw ${0:w}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i16 [[DATA]], i16 [[PORT]]) #[[ATTR3]], !srcloc [[META5:![0-9]+]]
+// CHECK-I386-NEXT:    ret i16 [[DATA]]
+//
+// CHECK-X64-LABEL: define dso_local noundef i16 @test_outpw(
+// CHECK-X64-SAME: i16 noundef [[PORT:%.*]], i16 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR1]] {
+// CHECK-X64-NEXT:  [[ENTRY:.*:]]
+// CHECK-X64-NEXT:    tail call void asm sideeffect "outw ${0:w}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i16 [[DATA]], i16 [[PORT]]) #[[ATTR5]], !srcloc [[META4:![0-9]+]]
+// CHECK-X64-NEXT:    ret i16 [[DATA]]
+//
+unsigned short test_outpw(unsigned short port, unsigned short data) {
+    return _outpw(port, data);
+}
+
+//
+// CHECK-I386-LABEL: define dso_local noundef i32 @test_outpd(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outl ${0:k}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR3]], !srcloc [[META6:![0-9]+]]
+// CHECK-I386-NEXT:    ret i32 [[DATA]]
+//
+// CHECK-X64-LABEL: define dso_local noundef i32 @test_outpd(
+// CHECK-X64-SAME: i16 noundef [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR1]] {
+// CHECK-X64-NEXT:  [[ENTRY:.*:]]
+// CHECK-X64-NEXT:    tail call void asm sideeffect "outl ${0:k}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR5]], !srcloc [[META5:![0-9]+]]
+// CHECK-X64-NEXT:    ret i32 [[DATA]]
+//
+unsigned long test_outpd(unsigned short port, unsigned long data) {
+    return _outpd(port, data);
+}
+
+//
+// CHECK-I386-LABEL: define dso_local noundef i32 @test_outp2(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR3]], !srcloc [[META4]]
+// CHECK-I386-NEXT:    ret i32 [[DATA]]
+//
+// CHECK-X64-LABEL: define dso_local noundef i32 @test_outp2(
+// CHECK-X64-SAME: i16 noundef [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR1]] {
+// CHECK-X64-NEXT:  [[ENTRY:.*:]]
+// CHECK-X64-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR5]], !srcloc [[META3]]
+// CHECK-X64-NEXT:    ret i32 [[DATA]]
+//
+int test_outp2(unsigned short port, int data) {
+    return outp(port, data);
+}
+
+//
+// CHECK-I386-LABEL: define dso_local noundef zeroext i16 @test_outpw2(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i16 noundef returned zeroext [[DATA:%.*]]) local_unnamed_addr #[[ATTR2]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outw ${0:w}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i16 [[DATA]], i16 [[PORT]]) #[[ATTR3]], !srcloc [[META5]]
+// CHECK-I386-NEXT:    ret i16 [[DATA]]
+//
+// CHECK-X64-LABEL: define dso_local noundef i16 @test_outpw2(
+// CHECK-X64-SAME: i16 noundef [[PORT:%.*]], i16 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR1]] {
+// CHECK-X64-NEXT:  [[ENTRY:.*:]]
+// CHECK-X64-NEXT:    tail call void asm sideeffect "outw ${0:w}, ${1:w}", "{ax},N{dx},~{memory},~{dirflag},~{fpsr},~{flags}"(i16 [[DATA]], i16 [[PORT]]) #[[ATTR5]], !srcloc [[META4]]
+// CHECK-X64-NEXT:    ret i16 [[DATA]]
+//
+unsigned short test_outpw2(unsigned short port, unsigned short data) {
+    return outpw(port, data);
+}
+
 #if defined(__x86_64__)
 
 char test__readgsbyte(unsigned long Offset) {

MalaySanghi · 2024-05-30T06:41:59Z

@FreddyLeaf @phoebewang please review

clang/lib/Headers/intrin.h

clang/test/CodeGen/X86/ms-x86-intrinsics.c

RKSimon

Maybe add a line to ReleaseNotes? But otherwise SGTM

MalaySanghi · 2024-06-18T07:12:37Z

Closing. The original request can be fulfilled with __outbyte/__outword/__outdword

Edit: The mentioned intrinsics are only declared, never implemented. That may be taken up as a follow-up. Currently reopening this PR.

phoebewang

LGTM.

phoebewang · 2024-05-30T08:16:05Z

clang/test/CodeGen/X86/ms-x86-intrinsics.c

@@ -63,6 +63,91 @@ unsigned __int64 test__emulu(unsigned int a, unsigned int b) {
 // CHECK: [[RES:%[0-9]+]] = mul nuw i64 [[Y]], [[X]]
 // CHECK: ret i64 [[RES]]

+//
+// CHECK-I386-LABEL: define dso_local noundef i32 @test_outp(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {


The PORT, DATA and ATTR2 are not necessary. You can just remove the whole line.

phoebewang · 2024-05-30T08:16:31Z

clang/test/CodeGen/X86/ms-x86-intrinsics.c

+//
+// CHECK-I386-LABEL: define dso_local noundef i32 @test_outp(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]


phoebewang · 2024-05-30T08:16:41Z

clang/test/CodeGen/X86/ms-x86-intrinsics.c

+// CHECK-I386-LABEL: define dso_local noundef i32 @test_outp(
+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR3:[0-9]+]], !srcloc [[META4:![0-9]+]]


phoebewang · 2024-05-30T08:16:46Z

clang/test/CodeGen/X86/ms-x86-intrinsics.c

+// CHECK-I386-SAME: i16 noundef zeroext [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
+// CHECK-I386-NEXT:  [[ENTRY:.*:]]
+// CHECK-I386-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR3:[0-9]+]], !srcloc [[META4:![0-9]+]]
+// CHECK-I386-NEXT:    ret i32 [[DATA]]


phoebewang · 2024-05-30T08:17:29Z

clang/test/CodeGen/X86/ms-x86-intrinsics.c

+// CHECK-X64-LABEL: define dso_local noundef i32 @test_outp(
+// CHECK-X64-SAME: i16 noundef [[PORT:%.*]], i32 noundef returned [[DATA:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// CHECK-X64-NEXT:  [[ENTRY:.*:]]
+// CHECK-X64-NEXT:    tail call void asm sideeffect "outb ${0:b}, ${1:w}", "{ax},N{dx},~{dirflag},~{fpsr},~{flags}"(i32 [[DATA]], i16 [[PORT]]) #[[ATTR5:[0-9]+]], !srcloc [[META3:![0-9]+]]
+// CHECK-X64-NEXT:    ret i32 [[DATA]]


Merge this with above and use CHECK only. The same for below.

phoebewang · 2024-05-31T08:37:00Z

clang/lib/Headers/intrin.h

@@ -348,6 +348,20 @@ static inline unsigned long _inpd(unsigned short port) {
  return ret;
 }

+static inline int _outp(unsigned short port, int data) {
+  __asm__ volatile("outb %b0, %w1" : : "a"(data), "Nd"(port));
+  return data;


Return the direct data seems useless. Did you check if MSVC returns the same value or it actually reads from the port and returns the old data?

phoebewang · 2024-06-19T05:39:08Z

clang/lib/Headers/intrin.h

@@ -348,6 +348,20 @@ static inline unsigned long _inpd(unsigned short port) {
  return ret;
 }

+static inline int _outp(unsigned short port, int data) {


Can we change it to __outbyte instead?

There's 2 differences between _outp and __outbyte.

First, the newer intrinsics don't return a value.

Second, __outbyte signature is

void __outbyte( unsigned short Port, unsigned char Data );

Note that the second input is unsigned char instead of int. This is likely because _outp is supposed to write a byte.

Other than that, I have verified that renaming to __outbyte works and is functionally equivalent.
When lowered via microsoft's cl, the asm is identical.

phoebewang

LGTM.

clang/lib/Headers/intrin.h

…vm#93774) A previous commit implemented _inp/w/d and renaming that to __inbyte/word/dword These intrinsics were declared in the header but never implemented.

llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels May 30, 2024

MalaySanghi changed the title ~~Add support for _outp{|w|d}~~ [X86]Add support for _outp{|w|d} May 30, 2024

phoebewang reviewed May 30, 2024

View reviewed changes

clang/lib/Headers/intrin.h Outdated Show resolved Hide resolved

clang/lib/Headers/intrin.h Outdated Show resolved Hide resolved

RKSimon reviewed May 30, 2024

View reviewed changes

clang/test/CodeGen/X86/ms-x86-intrinsics.c Outdated Show resolved Hide resolved

MalaySanghi mentioned this pull request May 30, 2024

[X86] Add support for MS inp functions. #93804

Merged

MalaySanghi added 3 commits May 30, 2024 23:57

Add support for _outp{|w|d}

17c3fc9

remove memory constraint and fix definition.

f0f4675

merge checks

794872b

MalaySanghi force-pushed the ms_outp branch from cf9d768 to 794872b Compare May 31, 2024 08:09

RKSimon reviewed May 31, 2024

View reviewed changes

MalaySanghi closed this Jun 18, 2024

MalaySanghi reopened this Jun 19, 2024

phoebewang approved these changes Jun 19, 2024

View reviewed changes

phoebewang reviewed Jun 19, 2024

View reviewed changes

implement byte/word/dword versions

4534c72

MalaySanghi changed the title ~~[X86]Add support for _outp{|w|d}~~ [X86]Add support for __outbyte/word/dword and __inbyte/word/dword Jun 20, 2024

phoebewang reviewed Jun 20, 2024

View reviewed changes

clang/lib/Headers/intrin.h Outdated Show resolved Hide resolved

replace volatile with __volatile__

b438d3f

phoebewang merged commit 3482403 into llvm:main Jun 21, 2024
7 checks passed

MalaySanghi deleted the ms_outp branch June 21, 2024 05:10

[X86]Add support for __outbyte/word/dword and __inbyte/word/dword #93774

[X86]Add support for __outbyte/word/dword and __inbyte/word/dword #93774

Uh oh!

Conversation

MalaySanghi commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MalaySanghi commented May 30, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

MalaySanghi commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[X86]Add support for outbyte/word/dword and inbyte/word/dword #93774

[X86]Add support for outbyte/word/dword and inbyte/word/dword #93774

MalaySanghi commented May 30, 2024 •

edited

Loading

llvmbot commented May 30, 2024 •

edited

Loading

MalaySanghi commented Jun 18, 2024 •

edited

Loading