Skip to content

Conversation

@artagnon
Copy link
Contributor

@artagnon artagnon commented Feb 3, 2025

For completeness, check poison, undef, and identity folding with GEP.

For completeness, check poison, undef, and identity folding with GEP.
@artagnon artagnon requested a review from nikic February 3, 2025 20:28
@github-actions
Copy link

github-actions bot commented Feb 3, 2025

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:
git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 95c0c784ac9a91a8e12331ad9574ac6ad75318b1 7c9a5e380c8af99bd09c9258d178f89230dd73e3 llvm/test/Other/constant-fold-gep.ll

The following files introduce new uses of undef:

  • llvm/test/Other/constant-fold-gep.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Did you confirm that none of these cases are tested elsewhere?

I'd prefer not to extend this non-UTC test unless necessary.

@artagnon
Copy link
Contributor Author

artagnon commented Feb 3, 2025

I looked at 1383cb6 ([ConstFold] Fix incorrect gep inbounds of undef fold), and it only seems to be tested indirectly in InstSimplify/ConstProp. As far as I can tell, there is no direct test, but I fully understand if you don't want to extend a non-UTC test.

@nikic
Copy link
Contributor

nikic commented Feb 3, 2025

InstSimplify/ConstProp is the usual way to test these code paths...

@artagnon artagnon closed this Feb 3, 2025
@artagnon artagnon deleted the constfold-gep-other-test branch February 3, 2025 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants