jdoerfert added a comment. I guess alignment is one of the main users of the old format, great to see it go :)
First set of comments. ================ Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:104 + return 1; + }; if (BOI.End - BOI.Begin > ABA_Argument) ---------------- I think this is a problem for things like `deref` which should default to 0? ================ Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:110 + if (BOI.End - BOI.Begin > ABA_Argument + 1) + Result.ArgValue = MinAlign(Result.ArgValue, GetArgOr1(1)); return Result; ---------------- Is this the min of the alignment and offset? If so, I'm not sure about min. Either way, can you clang format this and add a comment why alignment is special and how it looks? ================ Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:4196 LHS->setMetadata(LLVMContext::MD_nonnull, MD); - return eraseInstFromFunction(*II); + if (!HasOpBundles) + return eraseInstFromFunction(*II); ---------------- Can we split these changes off. Seem unrelated and easier. ================ Comment at: llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp:273 + if (!isValidAssumeForContext(ACall, J, DT)) + continue; Align NewDestAlignment = ---------------- I'm curious, why was this needed? ================ Comment at: llvm/test/Transforms/InstCombine/assume.ll:380 ; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i8 [[X:%.*]], 0 +; CHECK-NEXT: tail call void @llvm.assume(i1 false) ; CHECK-NEXT: tail call void @llvm.dbg.value(metadata i32 5, metadata !7, metadata !DIExpression()), !dbg !9 ---------------- Can you add the "beginning" of the operand bundles here so it becomes clear we do not have a no-op assume. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits