gchatelet added inline comments.
================ Comment at: clang/lib/CodeGen/CGCUDANV.cpp:491 new llvm::LoadInst(Var->getType(), ManagedVar, "ld.managed", false, - llvm::Align(Var->getAlignment()), I); + Var->getAlign().valueOrOne(), I); WorkItem.pop_back(); ---------------- tra wrote: > This appears to be a change in behavior. AFAICT, previously used > Var->getAlignment() could return alignment value or 0. Now it's value or 1. > > Is it intentional? The previous statement was constructing an `llvm::Align` from a value, and `llvm::Align` [asserts when the provided value is 0](https://github.com/llvm/llvm-project/blob/4ab2246d486ba30c4b2d654323a0c0b97565c0f1/llvm/include/llvm/Support/Alignment.h#L76-L81). This means that it is undefined to pass the value `0`. As far as `LoadInst` is concerned it can only accept valid alignments (i.e., powers of two => non zero). So you're right that it is not strictly NFC and that `*Var->getAlign()`would be a more rigorous transformation but I //think// that converting the `0` value to `1` moves us from UB to semantically valid code. I don't feel strongly about it though and I'm fine changing this to `*Var->getAlign()` to keep this patch NFC. WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142459/new/ https://reviews.llvm.org/D142459 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits