Skip to content

Conversation

@thewilsonator
Copy link
Contributor

No description provided.

@thewilsonator
Copy link
Contributor Author

Hmm

Attribute 'nocapture' does not apply to function return values
982: # |   %1 = call noalias nocapture align 1 ptr @_D24lifetime_local_variables__T11memoizeExprVAyaa21_756e69636f64652e41207c20756e69636f64652e4dZQCmUNbZSQDx12CodepointSet()

@kinke I think your fix #5015 was either wrong for old LLVM versions or it was already broken for old LLVM versions.

@calvin2021y
Copy link

add LDC_LLVM_VER to keep the old code for old LLVM versions will fix this?

@kinke
Copy link
Member

kinke commented Nov 12, 2025

I think your fix #5015 was either wrong for old LLVM versions or it was already broken for old LLVM versions.

I very much doubt so - we surely have many many RVO cases as part of the testsuite.

I'm not sure an extra test case (for NRVO and RVO) for these lifetime annotations are required. I mean you seem to have hit it quite immediately with LLVM master, without extra test case. And if you really want to cover this LLVM 22 limitation to allocas, you don't really have to check for elided lifetime annotations either - we'd most likely get LLVM errors/assertions [edit: well, before #5015] as soon as there are (N)RVO cases in an existing -femit-local-var-lifetime test file, without IR checks.

@kinke
Copy link
Member

kinke commented Nov 12, 2025

What I'd find more interesting test-wise is the handling of non-in-place-constructed temporaries. I suspect we don't declare their lifetime end at the end of the statement; and these temporaries could account for higher-than-needed stack pressure, perhaps/probably more than regular locals.

@thewilsonator
Copy link
Contributor Author

I'm not sure an extra test case (for NRVO and RVO) for these lifetime annotations are required.

Well these tests fail on old LLVMs so...

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.

3 participants