Confusion about: ASTContext::toCharUnitsFromBits usage

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Confusion about: ASTContext::toCharUnitsFromBits usage

Nathan Ridge via cfe-dev
Hi all,

ASTContext::toCharUnitsFromBits is implemented as follows in clang/lib/AST/ASTContext.cpp [1]:

---
  /// toCharUnitsFromBits - Convert a size in bits to a size in characters.
  CharUnits ASTContext::toCharUnitsFromBits(int64_t BitSize) const {
     return CharUnits::fromQuantity(BitSize / getCharWidth());
  }
---
 
Based on the comment, I would assume that the goal is to convert the BitSize into a number of characters
that can contain the number of bits.
The implementation though is rounding downwards, to the number of characters fitting in the BitSize.

On a number of places [2], the function is called with a guarantee that the BitSize is a multiple of the CharWidth.
On a recent change though [3], the patch depends on the downwards rounding behavior.

Is this downwards rounding behavior the expected behavior for this function ?

Thanks,

Jeroen Dobbelaere
----
[1] https://clang.llvm.org/doxygen/ASTContext_8cpp_source.html#l02161
[2] https://clang.llvm.org/doxygen/RecordLayoutBuilder_8cpp_source.html#l00769
      https://clang.llvm.org/doxygen/RecordLayoutBuilder_8cpp_source.html#l00781
[3] https://reviews.llvm.org/D63371


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Confusion about: ASTContext::toCharUnitsFromBits usage

Nathan Ridge via cfe-dev
On Thu, 20 Jun 2019 at 07:14, Jeroen Dobbelaere via cfe-dev <[hidden email]> wrote:
Hi all,

ASTContext::toCharUnitsFromBits is implemented as follows in clang/lib/AST/ASTContext.cpp [1]:

---
  /// toCharUnitsFromBits - Convert a size in bits to a size in characters.
  CharUnits ASTContext::toCharUnitsFromBits(int64_t BitSize) const {
     return CharUnits::fromQuantity(BitSize / getCharWidth());
  }
---

Based on the comment, I would assume that the goal is to convert the BitSize into a number of characters
that can contain the number of bits.
The implementation though is rounding downwards, to the number of characters fitting in the BitSize.

On a number of places [2], the function is called with a guarantee that the BitSize is a multiple of the CharWidth.
On a recent change though [3], the patch depends on the downwards rounding behavior.

Is this downwards rounding behavior the expected behavior for this function ?

We probably shouldn't be relying on that. Fixed in r364139.

Thanks,

Jeroen Dobbelaere
----
[1] https://clang.llvm.org/doxygen/ASTContext_8cpp_source.html#l02161
[2] https://clang.llvm.org/doxygen/RecordLayoutBuilder_8cpp_source.html#l00769
      https://clang.llvm.org/doxygen/RecordLayoutBuilder_8cpp_source.html#l00781
[3] https://reviews.llvm.org/D63371


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev