fhahn added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:2535 ArgAttrs[FirstIRArg + i] = llvm::AttributeSet::get(getLLVMContext(), Attrs); } ---------------- nlopes wrote: > fhahn wrote: > > nlopes wrote: > > > ab wrote: > > > > Hmm, if I'm reading this right, this overwrites the `nonnull > > > > dereferenceable align` attributes separately computed for `this` around > > > > l2335, right? (or `inalloca` and `sret` before that) > > > > > > > > It sounds like an ancient bug, that's only exposed because `noundef` > > > > ends up triggering this logic much more often? > > > > > > > > Many of our downstream tests hit this. The hacked up patch seems to do > > > > the job; ideally we'd feed the previously-computed attrs when > > > > constructing the AttrBuilder (which would also fix the padding > > > > argument), but we'd need to match up the IR args first. Maybe that's > > > > fine as a special-case for arg 0 though > > > nice catch! It's an ancient bug where arg 0 is overwritten. > > Is anybody looking into a fix or should we revert the patch until this can > > be properly addressed? > I would recommend against reverting this patch because of all the followup > patches. There were quite a few commits afterwards plus fixes to buildbot > configurations, so it's a non-trivial overhead to revert everything. > I was assuming @ab would fix it as he already root-caused the bug.. FWIW it seems a bit unfortunate that there are some clear regressions when the tests got update, e.g. https://github.com/llvm/llvm-project/commit/1b1c8d83d3567a60280291c0adb95d1d60335509#diff-7e80d593f26f6f6fb24765c6a169884d7350685d565ee970b0a7b9abaf0fb205L355 I'll work with @ab to fix this though rather than reverting, because another revert would cause even more conflicts for us downstream. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105169/new/ https://reviews.llvm.org/D105169 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits