rnk added inline comments.
================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 + llvm::APSInt Value = Enum->getInitVal(); + Value.setIsSigned(IsSigned); + Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); ---------------- dblaikie wrote: > Is the value already signed appropriately? > > Removing this line of code (and the `bool IsSigned` variable, so it doesn't > break `-Werror=unused-variable`) doesn't cause any tests to fail, that I can > see. I'm afraid it might not be NFC. I took the cautious approach of trying to leave things exactly as they were. Enums in C can have surprisingly different behavior, IIRC. ================ Comment at: llvm/lib/IR/DIBuilder.cpp:255 + assert(!Name.empty() && "Unable to create enumerator without name"); + return DIEnumerator::get(VMContext, APInt(Value), Value.isUnsigned(), Name); +} ---------------- mizvekov wrote: > Do I get it right, and this is not the first place that I noticed this, but > the terminology here is a bit unconventional with regards to "Signed" vs > "Negative"? > > It looks like around the debug info code, an APSInt will be a signed positive > value for representing a negative number, and an unsigned one to represent a > positive value. To my knowledge, we try not to store data in a sign-and-magnitude representation, it's always twos-complement. Meaning, APInt always stores a sequence of bits, typically interpreted as an unsigned positive integer. With outside information about the signedness of that data, you can answer the question of whether it is negative or not. APSInt adds the extra "signed" field. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106585/new/ https://reviews.llvm.org/D106585 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
