probinson added a comment.
We really do want to pack the four mutually exclusive cases into two bits. I
have tried to give more explicit comments inline to explain how you would do
this. It really should work fine, recognizing that the "not defaulted" case is
not explicitly represented in the textual IR because it uses a zero value in
the defaulted/deleted subfield of SPFlags.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1619
+ else {
+ SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+ }
----------------
SouraVX wrote:
> Previously SPFlagNotDefaulted is setted to SPFlagZero as it's normal value
> is, to save a bit. Hence in generated IR this flag is not getting set.
> instead 0 is getting emitted.
> As a result, test cases checking DISPFlagNotDefaulted in IR are failing.
Given that DISPFlagNotDefaulted is represented by the absence of the other
related flags, that makes sense. Those tests would verify the
DISPFlagNotDefaulted case by showing none of those flags are present.
================
Comment at: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp:60
// HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags:
DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
-// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags:
DIFlagPrototyped, spFlags: DISPFlagOptimized)
+// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags:
DIFlagPrototyped, spFlags: DISPFlagOptimized | DISPFlagNotDefaulted)
// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped
| DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
----------------
Because DISPFlagNotDefaulted has a zero value, the unmodified test correctly
verifies that no other defaulted/deleted flags are present.
================
Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:9
+
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic |
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic |
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
----------------
SouraVX wrote:
> SouraVX wrote:
> > This test case is failing, checking DISPFlagNotDefaulted.
> Please note here that, backend and llvm-dwarfdump is fine without this.
> Since it's value is '0' , we are able to query this using isNotDefaulted() --
> hence attribute
> DW_AT_defaulted having value DW_DEFAULTED_no is getting set and emitted and
> dumped fine by llvm-dwarfdump.
DISPFlagNotDefaulted is not explicitly represented in the textual IR; it is
implied by the absence of any of the other deleted/defaulted values. The test
needs to verify that spFlags is omitted from these DISubprogram entries; or if
there are other spFlags present, it must verify that the other
deleted/defaulted values are not present.
================
Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:93
+HANDLE_DISP_FLAG((1u << 10), DefaultedInClass)
+HANDLE_DISP_FLAG((1u << 11), DefaultedOutOfClass)
----------------
There are 4 mutually exclusive cases, which can be handled using 4 values in a
2-bit field. We will give NotDefaulted the zero value, so it is not explicitly
defined here. So we would have:
```
HANDLE_DISP_FLAG((1u << 9), Deleted)
HANDLE_DISP_FLAG((2u << 9), DefaultedInClass)
HANDLE_DISP_FLAG((3u << 9), DefaultedOutOfClass)
```
================
Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:98
// NOTE: Always must be equal to largest flag, check this when adding new
flags.
-HANDLE_DISP_FLAG((1 << 8), Largest)
+HANDLE_DISP_FLAG((1 << 11), Largest)
#undef DISP_FLAG_LARGEST_NEEDED
----------------
This can be 10, because we used only 2 bits for deleted/defaulted.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1615
SPFlagVirtuality = SPFlagVirtual | SPFlagPureVirtual,
+ SPFlagDefaultedInOrOutOfClass =
+ SPFlagDefaultedInClass | SPFlagDefaultedOutOfClass,
----------------
I would call this SPFlagDeletedOrDefaulted.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1632
+ static DISPFlags
+ toSPFlags(bool IsLocalToUnit, bool IsDefinition, bool IsOptimized,
+ unsigned Virtuality = SPFlagNonvirtual,
----------------
No, you don't want to modify this function. It is for converting from older
bitcode formats that did not have a DISPFlags field.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1777
+ }
+ bool isDeleted() const { return getSPFlags() & SPFlagDeleted; }
----------------
With all 4 values encoded in one field, isDeleted would become
```
return (getSPFlags() & SPFlagDefaultedOrDeleted) == SPFlagDeleted;
```
and of course the others would use the new mask name as well.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1309
+ dwarf::DW_DEFAULTED_no);
+ if (SP->isDeleted())
+ addFlag(SPDie, dwarf::DW_AT_deleted);
----------------
`else if` here. It cannot be both defaulted and deleted.
================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:603
case SPFlagVirtuality:
+ case SPFlagDefaultedInOrOutOfClass:
return "";
----------------
This would go away, if we pack 4 values into 2 bits.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68117/new/
https://reviews.llvm.org/D68117
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
