Fix parentheses for clarity in X86ISelLowering.cpp

[soumyaGhoshh]

fixes #121908

This pull request resolves issue #121908 by adding proper parentheses in X86ISelLowering.cpp to follow operator precedence rules. This change ensures better code clarity and avoids ambiguity.

This is my first contribution to LLVM. Please let me know if I missed any steps or made any mistakes. I’d greatly appreciate feedback to improve for future contributions!

Thank you for reviewing this pull request.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: None (soumyaGhoshh)

Changes

fixes #121908

This pull request resolves issue #121908 by adding proper parentheses in X86ISelLowering.cpp to follow operator precedence rules. This change ensures better code clarity and avoids ambiguity.

<img width="561" alt="Screenshot 2025-01-07 at 5 58 21 PM" src="https://github.com/user-attachments/assets/8d1594af-c2b6-4150-9bb2-b541654ed95e" />

This is my first contribution to LLVM. Please let me know if I missed any steps or made any mistakes. I’d greatly appreciate feedback to improve for future contributions!

Thank you for reviewing this pull request.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+1-1)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 68bdeb1cebeb9c..15408b13e44b6f 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -42418,7 +42418,7 @@ static SDValue combineTargetShuffle(SDValue N, const SDLoc &DL,
       // Canonicalize to VPERMV if both sources are the same.
       if (V1 == V2) {
         for (int &M : Mask)
-          M = (M < 0 ? M : M & Mask.size() - 1);
+          M = (M < 0 ? M : M & (Mask.size() - 1));
         SDValue NewMask = getConstVector(Mask, MaskVT, DAG, DL,
                                          /*IsMask=*/true);
         return DAG.getNode(X86ISD::VPERMV, DL, VT, NewMask, N.getOperand(0));

[RKSimon]

Have you confirmed the gcc -Wparentheses warning goes away?

[soumyaGhoshh]

Have you confirmed the gcc -Wparentheses warning goes away?

hello, mentioning that i am very beginner, i'm having compiling issues that says soemthing like MachineFucntion.h file not found include it

tried building the llvm in my machine still it having the issue and we not getting any warning but that error only, can you help ?

[RKSimon]

Have you been able to work through "Getting the Source Code and Building LLVM" section?

I'd recommend using something like: cmake -S llvm -B build -G Ninja -DCMAKE_C_FLAGS='-Wall -Wextra' -DCMAKE_CXX_FLAGS='-Wall -Wextra' -DLLVM_TARGETS_TO_BUILD="X86" to just build the X86 target

[soumyaGhoshh]

got it, very sorry i didn't read all through the building or contribution guides. I'm again checking for it. I built the whole LLVM locally which took a lot of time and resources u know, thanks for guiding with building only the target i''l try and revert to you

[soumyaGhoshh]

by the way while cloning my forked repo, will shallow cloning affect the merging with upstream repo in any way ? cause i tried cloning and it repeatedly failed for like 5 times on different stages

[RKSimon]

by the way while cloning my forked repo, will shallow cloning affect the merging with upstream repo in any way ? cause i tried cloning and it repeatedly failed for like 5 times on different stages

Sorry, I don't know - for dev work I always clone the entire thing

[soumyaGhoshh]

okay got it. i think i should give it a try again with cloning entirely

[phoebewang]

okay got it. i think i should give it a try again with cloning entirely

Although it's a good practice to learn how to build the entire LLVM, and usually it's necessary for committing a patch, you can sometimes verify the problem without entire build. E.g., https://godbolt.org/z/719WE69ex

[soumyaGhoshh]

okay got it. i think i should give it a try again with cloning entirely

Although it's a good practice to learn how to build the entire LLVM, and usually it's necessary for committing a patch, you can sometimes verify the problem without entire build. E.g., https://godbolt.org/z/719WE69ex

ahh wow that's a very good idea. it was super helpful thanks, could you tell how you know what specific type or version of compiler to use for checking or is it essentially the same for all?
also what you did is that check the line of code generating the warning from the large code file also we need to write the function and it's dependencies right ?

[phoebewang]

okay got it. i think i should give it a try again with cloning entirely

Although it's a good practice to learn how to build the entire LLVM, and usually it's necessary for committing a patch, you can sometimes verify the problem without entire build. E.g., https://godbolt.org/z/719WE69ex

ahh wow that's a very good idea. it was super helpful thanks, could you tell how you know what specific type or version of compiler to use for checking or is it essentially the same for all? also what you did is that check the line of code generating the warning from the large code file also we need to write the function and it's dependencies right ?

I usually use trunk to reproduce LLVM issues. For GCC related ones, I either use trunk or the latest release.

Right.

[RKSimon]

@soumyaGhoshh Were you successful in getting a working build? The patch LGTM so we can proceed with the merge, but if you're hoping to contribute again in the future you will need to get over these initial stages.

[soumyaGhoshh]

@soumyaGhoshh Were you successful in getting a working build? The patch LGTM so we can proceed with the merge, but if you're hoping to contribute again in the future you will need to get over these initial stages.

yh @RKSimon i think you can definitely go for it i checked and the warning goes away and the build works fine. thanks for your suggestion

[github-actions]

@soumyaGhoshh Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11937

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
addr=0x0000000000000268, file=  1, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
...