summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJay Kwak <82421531+jkwak-work@users.noreply.github.com>2025-06-06 10:48:06 -0700
committerGitHub <noreply@github.com>2025-06-06 17:48:06 +0000
commit7f04adbfb9dc0c39c372809ea02cc740d484b291 (patch)
treeb845bb73d027bef240cc6841d87dbc7290980944
parent2203df78332f155374866b172887d2568422ed76 (diff)
Disable Link-Time-Optimization by default (#7345)
* Disable Link-Time-Optimization by default LTO was requested for the release package a while ago. When we added it, LTO was enabled by default although it was needed only for the release packages. Later we found that the Release build cannot be incrementally recompiled when LTO is enabled. It sometimes works fine, but it required full recompilation when it doesn't work. We added a new CMake option, `SLANG_ENABLE_RELEASE_LTO`, to disable it for developers. But many Slang developers don't know the option exists. I was going to update the document, CONTRIBUTING.md, but I thought it will be better to change the default behavior. * Fix a compiler warning treated as an error on linux A padding variable was uninitialized, which is fine, but the compiler was complaining about it. * Fix other gcc error for uninitialized variable * Fix more compile warning treated as error * Fix compiler warning from gcc 11 It appears that this is a valid warning that the `delete this` is done on an offset 8 when the class uses multiple inheritance. The compiler warning is following: ``` In file included from /home/runner/work/slang/slang/source/core/slang-memory-file-system.h:5, from /home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp:3: In destructor ‘virtual Slang::ComBaseObject::~ComBaseObject()’, inlined from ‘uint32_t Slang::ComBaseObject::_releaseImpl()’ at /home/runner/work/slang/slang/source/core/slang-com-object.h:49:16, inlined from ‘virtual uint32_t Slang::MemoryFileSystem::release()’ at /home/runner/work/slang/slang/source/core/slang-memory-file-system.h:34:5: /home/runner/work/slang/slang/source/core/slang-com-object.h:33:31: error: ‘void operator delete(void*, std::size_t)’ called on pointer ‘<unknown>’ with nonzero offset 8 [-Werror=free-nonheap-object] 33 | virtual ~ComBaseObject() {} | ^ In destructor ‘virtual Slang::ComBaseObject::~ComBaseObject()’, inlined from ‘uint32_t Slang::ComBaseObject::_releaseImpl()’ at /home/runner/work/slang/slang/source/core/slang-com-object.h:49:16, inlined from ‘virtual uint32_t Slang::MemoryFileSystem::release()’ at /home/runner/work/slang/slang/source/core/slang-memory-file-system.h:34:5, inlined from ‘Slang::ComPtr<T>::~ComPtr() [with T = ISlangMutableFileSystem]’ at /home/runner/work/slang/slang/include/slang-com-ptr.h:113:34, inlined from ‘void _modulePtr_impl(UnitTestContext*)’ at /home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp:92:1: /home/runner/work/slang/slang/source/core/slang-com-object.h:33:31: error: ‘void operator delete(void*, std::size_t)’ called on pointer ‘<unknown>’ with nonzero offset 8 [-Werror=free-nonheap-object] 33 | virtual ~ComBaseObject() {} | ^ /home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp: In function ‘void _modulePtr_impl(UnitTestContext*)’: /home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp:36:69: note: returned from ‘void* operator new(std::size_t)’ 36 | ComPtr<ISlangMutableFileSystem>(new Slang::MemoryFileSystem()); | ^ ``` The problem is on the fact that `ComBaseObject` is not the first in the multiple inheritance: ``` class MemoryFileSystem : public ISlangMutableFileSystem, public ComBaseObject { public: // ISlangUnknown SLANG_COM_BASE_IUNKNOWN_ALL ``` It should be: ``` class MemoryFileSystem : public ComBaseObject, public ISlangMutableFileSystem ``` The chain of ComObject release is little complicated and it is easy to make a mistake. Here is summary with details, 1. `release()` is declared as a pure-virtual in ISlangUnknown, which is one of the base classes of `ISlangMutableFileSystem`. ``` struct ISlangUnknown { virtual SLANG_NO_THROW uint32_t SLANG_MCALL release() = 0; ``` 2. `release()` is implemented with the macro `SLANG_COM_BASE_IUNKNOWN_RELEASE`. ``` SLANG_NO_THROW uint32_t SLANG_MCALL release() SLANG_OVERRIDE \ { \ return _releaseImpl(); \ } inline uint32_t ComBaseObject::_releaseImpl() { // Check there is a ref count to avoid underflow SLANG_ASSERT(m_refCount != 0); const uint32_t count = --m_refCount; if (count == 0) { delete this; } return count; } ``` 3. The instance of `MemoryFileSystem` is handled by ComPtr. And `ComPtr::~ComPtr()` calls the `release()`. ``` ComPtr<ISlangMutableFileSystem> memoryFileSystem = ComPtr<ISlangMutableFileSystem>(new Slang::MemoryFileSystem()); SLANG_FORCE_INLINE ~ComPtr() { if (m_ptr) ((Ptr)m_ptr)->release(); } ``` 4. When `delete this` is called, because ComBaseObject is not the first in the multiple inheritance, `this` is 8 byte off from the actual instance address. A fix for this is to change the order of the inheritance and make ComBaseObject to be the first in the order.
-rw-r--r--.github/workflows/release.yml1
-rw-r--r--CMakeLists.txt2
-rw-r--r--docs/building.md2
-rw-r--r--source/core/slang-blob.h2
-rw-r--r--source/core/slang-file-system.h4
-rw-r--r--source/core/slang-memory-file-system.h2
-rw-r--r--source/core/slang-shared-library.h2
-rw-r--r--source/core/slang-zip-file-system.cpp6
-rw-r--r--source/slang-llvm/slang-llvm.cpp2
-rw-r--r--source/slang/slang-emit-vm.cpp2
-rw-r--r--source/slang/slang-serialize-ast.cpp6
-rw-r--r--source/slang/slang-serialize.h4
-rw-r--r--source/slang/slang-vm-bytecode.cpp8
-rw-r--r--source/slang/slang-workspace-version.h2
14 files changed, 23 insertions, 22 deletions
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index f2cf299c8..5319fa373 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -106,6 +106,7 @@ jobs:
cmake --preset default --fresh \
-DSLANG_GENERATORS_PATH=build-platform-generators/bin \
-DSLANG_ENABLE_EXAMPLES=OFF \
+ -DSLANG_ENABLE_RELEASE_LTO=ON \
"-DSLANG_SLANG_LLVM_FLAVOR=$(
[[ "${{matrix.build-slang-llvm}}" = "true" ]] && echo "USE_SYSTEM_LLVM" || echo "DISABLE")"
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 98eb4d9c3..2c4f8d410 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -275,7 +275,7 @@ option(
ON
)
-option(SLANG_ENABLE_RELEASE_LTO "Enable LTO for Release builds" ON)
+option(SLANG_ENABLE_RELEASE_LTO "Enable LTO for Release builds" OFF)
option(
SLANG_ENABLE_SPLIT_DEBUG_INFO
diff --git a/docs/building.md b/docs/building.md
index de76d2825..3a29a6335 100644
--- a/docs/building.md
+++ b/docs/building.md
@@ -166,7 +166,7 @@ See the [documentation on testing](../tools/slang-test/README.md) for more infor
| `SLANG_ENABLE_EXAMPLES` | `TRUE` | Enable example targets, requires SLANG_ENABLE_GFX |
| `SLANG_LIB_TYPE` | `SHARED` | How to build the slang library |
| `SLANG_ENABLE_RELEASE_DEBUG_INFO` | `TRUE` | Enable generating debug info for Release configs |
-| `SLANG_ENABLE_RELEASE_LTO` | `TRUE` | Enable LTO for Release builds |
+| `SLANG_ENABLE_RELEASE_LTO` | `FALSE` | Enable LTO for Release builds |
| `SLANG_ENABLE_SPLIT_DEBUG_INFO` | `TRUE` | Enable generating split debug info for Debug and RelWithDebInfo configs |
| `SLANG_SLANG_LLVM_FLAVOR` | `FETCH_BINARY_IF_POSSIBLE` | How to set up llvm support |
| `SLANG_SLANG_LLVM_BINARY_URL` | System dependent | URL specifying the location of the slang-llvm prebuilt library |
diff --git a/source/core/slang-blob.h b/source/core/slang-blob.h
index af3206fb5..5277ef0cd 100644
--- a/source/core/slang-blob.h
+++ b/source/core/slang-blob.h
@@ -15,7 +15,7 @@ namespace Slang
/** Base class for simple blobs.
*/
-class BlobBase : public ISlangBlob, public ICastable, public ComBaseObject
+class BlobBase : public ComBaseObject, public ISlangBlob, public ICastable
{
public:
// ISlangUnknown
diff --git a/source/core/slang-file-system.h b/source/core/slang-file-system.h
index 7ed500eb9..06d647c80 100644
--- a/source/core/slang-file-system.h
+++ b/source/core/slang-file-system.h
@@ -97,7 +97,7 @@ NOTE! That this behavior is the same as previously in that....
1) calcRelativePath, just returns the path as processed by the Path:: methods
2) getUniqueIdentity behavior depends on the UniqueIdentityMode.
*/
-class CacheFileSystem : public ISlangFileSystemExt, public ComBaseObject
+class CacheFileSystem : public ComBaseObject, public ISlangFileSystemExt
{
public:
SLANG_CLASS_GUID(0x2f4d1d03, 0xa0d1, 0x434b, {0x87, 0x7a, 0x65, 0x5, 0xa4, 0xa0, 0x9a, 0x3b})
@@ -268,7 +268,7 @@ protected:
OSPathKind m_osPathKind = OSPathKind::None; ///< OS path kind
};
-class RelativeFileSystem : public ISlangMutableFileSystem, public ComBaseObject
+class RelativeFileSystem : public ComBaseObject, public ISlangMutableFileSystem
{
public:
SLANG_COM_BASE_IUNKNOWN_ALL
diff --git a/source/core/slang-memory-file-system.h b/source/core/slang-memory-file-system.h
index ebf6e239a..84ce6a244 100644
--- a/source/core/slang-memory-file-system.h
+++ b/source/core/slang-memory-file-system.h
@@ -27,7 +27,7 @@ TODO(JS):
* We may want to make saveFile take a blob, or have a version that does. Doing so would allow the
application to handle memory management around the blob.
*/
-class MemoryFileSystem : public ISlangMutableFileSystem, public ComBaseObject
+class MemoryFileSystem : public ComBaseObject, public ISlangMutableFileSystem
{
public:
// ISlangUnknown
diff --git a/source/core/slang-shared-library.h b/source/core/slang-shared-library.h
index fbc8a1d30..8128eee05 100644
--- a/source/core/slang-shared-library.h
+++ b/source/core/slang-shared-library.h
@@ -48,7 +48,7 @@ private:
static DefaultSharedLibraryLoader s_singleton;
};
-class DefaultSharedLibrary : public ISlangSharedLibrary, public ComBaseObject
+class DefaultSharedLibrary : public ComBaseObject, public ISlangSharedLibrary
{
public:
SLANG_CLASS_GUID(0xe7f2597b, 0xf803, 0x4b6e, {0xaf, 0x8b, 0xcb, 0xe3, 0xa2, 0x21, 0xfd, 0x5a})
diff --git a/source/core/slang-zip-file-system.cpp b/source/core/slang-zip-file-system.cpp
index 9c805dc55..0d3503222 100644
--- a/source/core/slang-zip-file-system.cpp
+++ b/source/core/slang-zip-file-system.cpp
@@ -15,9 +15,9 @@
namespace Slang
{
-class ZipFileSystemImpl : public ISlangMutableFileSystem,
- public IArchiveFileSystem,
- public ComBaseObject
+class ZipFileSystemImpl : public ComBaseObject,
+ public ISlangMutableFileSystem,
+ public IArchiveFileSystem
{
public:
// ISlangUnknown
diff --git a/source/slang-llvm/slang-llvm.cpp b/source/slang-llvm/slang-llvm.cpp
index e645fb3c7..2860874b5 100644
--- a/source/slang-llvm/slang-llvm.cpp
+++ b/source/slang-llvm/slang-llvm.cpp
@@ -175,7 +175,7 @@ public:
/* This implementation uses atomic ref counting to ensure the shared libraries lifetime can outlive
the LLVMDownstreamCompileResult and the compilation that created it */
-class LLVMJITSharedLibrary : public ISlangSharedLibrary, public ComBaseObject
+class LLVMJITSharedLibrary : public ComBaseObject, public ISlangSharedLibrary
{
public:
// ISlangUnknown
diff --git a/source/slang/slang-emit-vm.cpp b/source/slang/slang-emit-vm.cpp
index fc0b4432e..772d9f84a 100644
--- a/source/slang/slang-emit-vm.cpp
+++ b/source/slang/slang-emit-vm.cpp
@@ -260,7 +260,7 @@ public:
uint32_t extOp,
ArrayView<VMOperand> operands)
{
- VMInstHeader instHeader;
+ VMInstHeader instHeader = {};
instHeader.opcode = op;
instHeader.opcodeExtension = extOp;
instHeader.operandCount = (uint16_t)operands.getCount();
diff --git a/source/slang/slang-serialize-ast.cpp b/source/slang/slang-serialize-ast.cpp
index 02dd374c1..9fa338449 100644
--- a/source/slang/slang-serialize-ast.cpp
+++ b/source/slang/slang-serialize-ast.cpp
@@ -136,7 +136,7 @@ void serialize(Serializer const& serializer, SemanticVersion& value)
void serialize(Serializer const& serializer, SyntaxClass<NodeBase>& value)
{
- ASTNodeType raw;
+ ASTNodeType raw = ASTNodeType(0);
if (isWriting(serializer))
{
raw = value.getTag();
@@ -277,7 +277,7 @@ void serialize(Serializer const& serializer, CapabilityAtomSet& value)
{
while (hasElements(serializer))
{
- CapabilityAtom atom;
+ CapabilityAtom atom = CapabilityAtom(0);
serialize(serializer, atom);
value.add(UInt(atom));
}
@@ -769,7 +769,7 @@ void ASTDecodingContext::handleASTNode(NodeBase*& outNode)
{
ASTSerializer serializer(this);
- ASTNodeType typeTag;
+ ASTNodeType typeTag = ASTNodeType(0);
serialize(serializer, typeTag);
switch (_getPseudoASTNodeType(typeTag))
{
diff --git a/source/slang/slang-serialize.h b/source/slang/slang-serialize.h
index b962ee2b7..591f43139 100644
--- a/source/slang/slang-serialize.h
+++ b/source/slang/slang-serialize.h
@@ -859,7 +859,7 @@ void serialize(S const& serializer, Dictionary<K, V>& value)
value.clear();
while (hasElements(serializer))
{
- KeyValuePair<K, V> pair;
+ KeyValuePair<K, V> pair{K(), V()};
serialize(serializer, pair);
value.add(pair.key, pair.value);
}
@@ -880,7 +880,7 @@ void serialize(S const& serializer, OrderedDictionary<K, V>& value)
value.clear();
while (hasElements(serializer))
{
- KeyValuePair<K, V> pair;
+ KeyValuePair<K, V> pair{K(), V()};
serialize(serializer, pair);
value.add(pair.key, pair.value);
}
diff --git a/source/slang/slang-vm-bytecode.cpp b/source/slang/slang-vm-bytecode.cpp
index 1eafa4e57..bf16805b8 100644
--- a/source/slang/slang-vm-bytecode.cpp
+++ b/source/slang/slang-vm-bytecode.cpp
@@ -328,28 +328,28 @@ void printVMInst(StringBuilder& sb, VMModuleView* moduleView, VMInstHeader* inst
{
case OperandDataType::Int32:
{
- int32_t val;
+ int32_t val = 0;
moduleView->getConstant<int32_t>(operand, val);
sb << "i32(" << val << ")";
continue;
}
case OperandDataType::Int64:
{
- int64_t val;
+ int64_t val = 0;
moduleView->getConstant<int64_t>(operand, val);
sb << "i64(" << val << ")";
continue;
}
case OperandDataType::Float32:
{
- float val;
+ float val = 0.f;
moduleView->getConstant<float>(operand, val);
sb << "f32(" << val << ")";
continue;
}
case OperandDataType::Float64:
{
- double val;
+ double val = 0.0;
moduleView->getConstant<double>(operand, val);
sb << "f32(" << val << ")";
continue;
diff --git a/source/slang/slang-workspace-version.h b/source/slang/slang-workspace-version.h
index 711504afb..1a12a2b42 100644
--- a/source/slang/slang-workspace-version.h
+++ b/source/slang/slang-workspace-version.h
@@ -159,7 +159,7 @@ struct OwnedPreprocessorMacroDefinition
String name;
String value;
};
-class Workspace : public ISlangFileSystem, public ComObject
+class Workspace : public ComObject, public ISlangFileSystem
{
private:
RefPtr<WorkspaceVersion> currentVersion;