Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Flang2] Fix duplicate TBAA type systems across modules #1460

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

1997alireza
Copy link

@1997alireza 1997alireza commented Nov 6, 2024

Currently, functions within the same module are assigned unique TBAA type system IDs based on their order, but functions in different modules can unintentionally receive the same ID, leading to incorrect non-aliasing results. This patch resolves the issue by appending a hash of the module name to each function type system metadata, ensuring unique type systems across modules and preventing aliasing errors. This resolves #1458.

    In Flang, functions within the same module are assigned unique TBAA type system IDs based on their order, but functions in different modules can unintentionally receive the same ID, leading to incorrect non-aliasing results. This commit resolves the issue by appending a hash of the module name to each function type system metadata, ensuring unique type systems across modules and preventing aliasing errors.
@@ -2643,7 +2644,7 @@ get_omnipotent_pointer(LL_Module *module)
const char *baseName = "Flang FAA";
const char *const omniName = "unlimited ptr";
const char *const unObjName = "unref ptr";
snprintf(baseBuff, 32, "%s %x", baseName, funcId);
snprintf(baseBuff, 32, "%s %zx %x", baseName, std::hash<std::string>{}(current_module->module_name), funcId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you format this line according to LLVM Coding Standards?

@@ -43,6 +43,7 @@
#include "symfun.h"
#include "ilidir.h"
#include "fdirect.h"
#include <string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this build error, this #include statement should be placed above line 18 (#include "machreg.h"). In fact, all standard headers should be included before custom headers, including stdlib.h, stdio.h, etc.

@@ -0,0 +1,18 @@
! This test contians two files, tbaa_multimod_01_caller.f90 and tbaa_multimod_01_callee.f90
! RUN: %flang -Wl,-mllvm,-aa-trace -fuse-ld=lld -flto=full -O3 %s %S/tbaa_multimod_01_callee.f90 -o - 2>&1 1>- | FileCheck %s
Copy link
Collaborator

@bryanpkc bryanpkc Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are failing in the CI jobs. I understand that -aa-trace only works when LLVM has assertions enabled, but I believe that the GitHub workflow for classic-flang-llvm-project already configures LLVM with -DLLVM_ENABLE_ASSERTIONS=ON:

  1. https://github.com/flang-compiler/classic-flang-llvm-project/blob/release_17x/.github/workflows/pre-compile_llvm.yml#L65

  2. https://github.com/flang-compiler/classic-flang-llvm-project/blob/release_17x/.github/workflows/UbuntuDockerfile#L54

@1997alireza Can you build classic-flang-llvm-project using the same commands as used by the CI jobs, and confirm whether you can reproduce the test failures in your environment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect no alias by TBAA due to a problem with metadata tags
3 participants