[Clang] Condition is always true which is caused by a possible copy-pasted bug in CGOpenMPRuntime.cpp

[ustchcs-bugfinder]

llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp

Line 11173 in 7b338a6

if (Kind == ParamKindTy::LinearUVal || ParamKindTy::LinearRef)

kind == is likely omitted. Therefore the condition on L11173 is always true.

Correct code:

if (Kind == ParamKindTy::LinearUVal ||  kind == ParamKindTy::LinearRef)   // Line 11173

Current code:

/// Maps To Vector (MTV), as defined in 4.1.1 of the AAVFABI (2021Q1).
static bool getAArch64MTV(QualType QT, ParamKindTy Kind) {
  QT = QT.getCanonicalType();

  if (QT->isVoidType())
    return false;

  if (Kind == ParamKindTy::Uniform)
    return false;

  if (Kind == ParamKindTy::LinearUVal || ParamKindTy::LinearRef)   // Line 11173
    return false;

  if ((Kind == ParamKindTy::Linear || Kind == ParamKindTy::LinearVal) &&
      !QT->isReferenceType())
    return false;

  return true;
}

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[llvmbot]

@llvm/issue-subscribers-openmp

[shiltian]

Yes, thanks for pointing out. Do you want to submit a patch?

[ustchcs-bugfinder]

You are welcome. We are glad to know this issue is useful to the community. That's enough.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

[samuelmaina]

@SamTebbs33 , Do I have to modify the failing tests and create new ones to regress for the added conditions?

[SamTebbs33]

I'm not familiar with that file, so I'm not sure what tests are failing but you can try fixing the condition and changing any tests that need to be changed as as a result of that.

[s-pratik]

Hi, I want to take this issue. As I am a beginner I just wanted to know how can I run the test cases after fixing the condition.

[samuelmaina]

Hi @s-pratik I am working on this issue.No one is assigning issues as far as I know.Just pick any as if it's not closed.

[jhuber6]

Hi, I want to take this issue. As I am a beginner I just wanted to know how can I run the test cases after fixing the condition.

You can run the check-clang target in the build directory, assuming you've built clang like in https://llvm.org/docs/CMake.html. E.g. ninja check-clang if you're using Ninja. The tests use llvm-lit https://llvm.org/docs/CommandGuide/lit.html which should be present in your build directory's ./bin directory. You can run a single test manually if you do not with to use the target. For example:

./bin/llvm-lit -vv ../clang/test/Driver/clang_cpp.c

Generally, if all the tests still pass after making a change you can try modifying the tests to catch the new change.

[samuelmaina]

@jhuber6 How do I get which tests are failing? I have changed some tests to test the current condition but I can't see the details of which tests are failing, I am only getting a summary. I have tried the -v (--verbose) flag but I can't get the tests .How can I get the files and the details?
Here is the is code
`llvm-lit: /home/samuelmayna/llvm-project/llvm/utils/lit/lit/llvm/config.py:459: note: using clang: /home/samuelmayna/llvm-project/build/bin/clang

Testing Time: 2577.47s
Skipped : 8
Unsupported : 134
Passed : 33312
Expectedly Failed: 27`

[jhuber6]

If you get that final summary, that means none of the tests failed. This bug doesn't trigger any existing tests, so you would need to either create or modify a test. I don't know if there's an existing test that covers something similar, it's usually a good place to start.

[samuelmaina]

@jhuber6 The first unit tests I could find are for functions that are 6 levels higher from this function .The function is very low level and couldn't find a way to affect it from the higher functions.

[jhuber6]

This fix is extremely minor. Adding a new test that stimulates it will be difficult. Personally I wouldn't bother making a new test.

[samuelmaina]

Thanks @jhuber6 . Can you please approve the patch at https://reviews.llvm.org/D146370? I am in outreachy intern and I need to document an approved patch.

[xgupta]

Fixed by -c1ab198.