Skip to content

Conversation

romainthomas
Copy link
Contributor

Currently, the semantic of X86's add instructions is not represented in MCInstrDesc::isAdd.

This commit adds this support by adding the isAdd = 1 flag in tablegen definition.

Currently, the semantic of X86's add instructions is not represented in
`MCInstrDesc::isAdd`.

This commit adds this support by adding the `isAdd = 1` flag in tablegen
definition.
@llvmbot
Copy link
Member

llvmbot commented Dec 7, 2024

@llvm/pr-subscribers-backend-x86

Author: Romain Thomas (romainthomas)

Changes

Currently, the semantic of X86's add instructions is not represented in MCInstrDesc::isAdd.

This commit adds this support by adding the isAdd = 1 flag in tablegen definition.


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrArithmetic.td (+9-4)
diff --git a/llvm/lib/Target/X86/X86InstrArithmetic.td b/llvm/lib/Target/X86/X86InstrArithmetic.td
index 16ca2882a84daf..4a9b16aac81d56 100644
--- a/llvm/lib/Target/X86/X86InstrArithmetic.td
+++ b/llvm/lib/Target/X86/X86InstrArithmetic.td
@@ -1080,8 +1080,11 @@ defm OR  : ArithBinOp_RF<0x09, 0x0B, 0x0D, "or", MRM1r, MRM1m,
                          X86or_flag, or, 1, 0, 0>;
 defm XOR : ArithBinOp_RF<0x31, 0x33, 0x35, "xor", MRM6r, MRM6m,
                          X86xor_flag, xor, 1, 0, 0>;
-defm ADD : ArithBinOp_RF<0x01, 0x03, 0x05, "add", MRM0r, MRM0m,
-                         X86add_flag, add, 1, 1, 1>;
+let isAdd = 1 in {
+  defm ADD : ArithBinOp_RF<0x01, 0x03, 0x05, "add", MRM0r, MRM0m,
+                           X86add_flag, add, 1, 1, 1>;
+}
+
 let isCompare = 1 in {
   defm SUB : ArithBinOp_RF<0x29, 0x2B, 0x2D, "sub", MRM5r, MRM5m,
                            X86sub_flag, sub, 0, 1, 0>;
@@ -1097,8 +1100,10 @@ let isCodeGenOnly = 1, hasSideEffects = 0, Constraints = "$src1 = $dst",
                        Sched<[WriteALU]>;
 
 // Arithmetic.
-defm ADC : ArithBinOp_RFF<0x11, 0x13, 0x15, "adc", MRM2r, MRM2m, X86adc_flag,
-                          1, 0>;
+let isAdd = 1 in {
+  defm ADC : ArithBinOp_RFF<0x11, 0x13, 0x15, "adc", MRM2r, MRM2m, X86adc_flag,
+                            1, 0>;
+}
 defm SBB : ArithBinOp_RFF<0x19, 0x1B, 0x1D, "sbb", MRM3r, MRM3m, X86sbb_flag,
                           0, 0>;
 

@topperc
Copy link
Collaborator

topperc commented Dec 7, 2024

Tests?

@topperc
Copy link
Collaborator

topperc commented Dec 7, 2024

Tests?

Nevermind I guess you can't test it because the MCInstrDesc::isAdd is only used in 2 places in target specific code.

lib/Target/AMDGPU/SIRegisterInfo.cpp
852:  assert(MI.getDesc().isAdd());

lib/Target/Hexagon/HexagonHardwareLoops.cpp
443:      if (DI->getDesc().isAdd()) {
1624:      if (DI->getDesc().isAdd()) {

Copy link
Contributor

@antoniofrighetto antoniofrighetto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's wait for another feedback though.

@@ -1097,8 +1100,10 @@ let isCodeGenOnly = 1, hasSideEffects = 0, Constraints = "$src1 = $dst",
Sched<[WriteALU]>;

// Arithmetic.
defm ADC : ArithBinOp_RFF<0x11, 0x13, 0x15, "adc", MRM2r, MRM2m, X86adc_flag,
1, 0>;
let isAdd = 1 in {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without knowing what the flag is used for, it's hard to say whether an Add with carry input should be consider an add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants