Conversation
2b62ac9 to
983f779
Compare
This triggers when building openssl in cheerpos mode.
There was a problem hiding this comment.
Pull request overview
This PR addresses build/runtime issues (notably seen with OpenSSL) in the Cheerp-modified LLVM tree by tightening SROA legality checks, relaxing an overly strict debug assertion in registerization, and teaching identical code folding how to compare fence instructions.
Changes:
- Prevent SROA integer-widening from proceeding on non-byte-addressable targets when it would require creating illegal integer widths (e.g.
i24) for mem-intrinsic slices. - Stop
VertexColorer::iterativeDeepeningfrom asserting when called with an exhaustedIterationsCounter. - Add
Instruction::Fencehandling toIdenticalCodeFolding::equivalentInstruction.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| llvm/lib/Transforms/Scalar/SROA.cpp | Adds a legality gate to avoid creating illegal integer slice widths in NBA (non-byte-addressable) mode. |
| llvm/lib/CheerpUtils/Registerize.cpp | Disables a debug assertion that can fire when the iteration budget is zero. |
| llvm/lib/CheerpUtils/IdenticalCodeFolding.cpp | Adds equivalence logic for fence instructions (ordering + sync scope). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lowerBound = lowerBoundOnNumberOfColors(/*forceEvaluation*/true); | ||
| } | ||
| assert(counter.remaining() > 0); | ||
| //assert(counter.remaining() > 0); |
There was a problem hiding this comment.
Leaving the old assertion commented out makes the intent unclear and leaves dead code behind. Prefer either removing it entirely or replacing it with an explicit guard/early return when counter.remaining() == 0 (and, if useful, a short comment explaining why 0 iterations is a valid input).
| //assert(counter.remaining() > 0); |
| uint64_t SliceBits = (RelEnd - RelBegin) * 8; | ||
| // CHEERP: disallow weird integer sizes like i24 from being created | ||
| if (!DL.isByteAddressable() && !DL.isLegalInteger(SliceBits)) | ||
| return false; |
There was a problem hiding this comment.
Consider adding a regression test for the new non-byte-addressable DataLayout behavior here (e.g., a module with datalayout including 'b' and a 3-byte memintrinsic slice) to ensure SROA no longer tries integer widening that would require creating illegal integer types like i24.
No description provided.