llvm / clangir

A new (MLIR based) high-level IR for clang.
https://clangir.org
Other
388 stars 99 forks source link

[CIR][CIRGen][TBAA] Initial TBAA support #1116

Closed PikachuHyA closed 1 week ago

PikachuHyA commented 2 weeks ago

This is the first patch to support TBAA, following the discussion at https://github.com/llvm/clangir/pull/1076#discussion_r1835031415

PikachuHyA commented 1 week ago

Summary of Changes

Explanations

bcardosolopes commented 1 week ago
  • In this patch, we use mlir::ArrayAttr to simplify the TBAA attribute, allowing for a quicker landing of the patch. We will refine the TBAA attribute design in the next patch.

I dislike optional because there are two levels on null checking here (the optional could be empty and the array could also be empty), which is quite confusing. Better yet that LLVM doesn't really support multiple entries. I suggested to you in previous comments to create a new attr if necessary to wrap the array exactly for this reason: the array becomes an implementation detail and it doesn't really matter if we remove the array later, since it's internal state of the attribute.

Anyways, I'm fine if this comes as incremental work though. Thanks