[PATCH] D109620: [APInt] Add a concat method, use LLVM_UNLIKELY to help optimizer.

[Chris Lattner via Phabricator via llvm-commits]

lattner marked an inline comment as done.
lattner added inline comments.


================
Comment at: llvm/include/llvm/ADT/APInt.h:884
+  ///   (this->zext(NewWidth) << NewLSB.getBitWidth()) | NewLSB.zext(NewWidth)
+  APInt concat(const APInt &NewLSB) const {
+    /// If the result will be small, then both the merged values are small.
----------------
foad wrote:
> Is there a reason for the argument to be NewLSB instead of NewMSB?
> 
> If there was a non-member form would it be `static APInt concat(const APInt &MSB, const APInt &LSB)`? I would slightly prefer a `(LSB, MSB)` argument order, but I guess there is no real consistency in existing APIs.
Yes, check out the unit tests.  I actually started this way, but it was too weird for:

`APInt(0xA).concat(APInt(0xB)).concat(APInt(0xC)) == 0xCBA`

Given how dot syntax works, it is better for this to be NewLSB.


================
Comment at: llvm/lib/Support/APInt.cpp:345
+///   (this->zext(NewWidth) << NewLSB.getBitWidth()) | NewLSB.zext(NewWidth)
+/// In the slow case, we know the result is larger.
+APInt APInt::concatSlowCase(const APInt &NewLSB) const {
----------------
foad wrote:
> Larger than what? Did you mean "large"?
Yes, thank you for catching that.  Fixed to "large".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109620/new/

https://reviews.llvm.org/D109620