rnk added inline comments.
================ Comment at: lib/Sema/SemaStmtAsm.cpp:674 + Expr::EvalResult EvlResult; + // Try to evaluate the identifier as enum constant + if (isa<clang::EnumType>(Res->getType()) && Res->EvaluateAsRValue(EvlResult, ---------------- Please add a comment explaining that non-enum integer constants should not be folded. I expected that MSVC would fold constexpr integers here, but it does not, so that's worth noting. ================ Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12 + const int a = 0; + // CHECK-NOT: mov eax, [$$0] + __asm mov eax, [a] ---------------- mharoush wrote: > rnk wrote: > > Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing > > ms-inline-asm.c tests do so that this test is easier to debug when it fails. > This test case was just meant verify that other Integer constants are not > folded since we get a different behavior for statements such as mov eax, [a]. > #--- > In this example X86AsmParser regards the address of the variable 'a' and not > its value i.e. we end up with the value of 'a' in eax (loaded from the stack) > and not with the value pointed by the const int value of 'a' as its address. > ---# > > I can clarify the intention in a comment or completely remove the test case > since this isn't really required here. The test case is fine and the intention is clear, I am just suggesting that you use more FileCheck features (CHECK-LABEL and CHECK-SAME) to make the test easier to debug when it fails in the future. Repository: rL LLVM https://reviews.llvm.org/D33277 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
