LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 1164 - CBE can generate name conflicts
Summary: CBE can generate name conflicts
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Backend: C (show other bugs)
Version: trunk
Hardware: All All
: P normal
Assignee: Bill Wendling
URL:
Keywords: miscompilation, regression
Depends on:
Blocks:
 
Reported: 2007-02-03 17:33 PST by Chris Lattner
Modified: 2010-02-22 12:44 PST (History)
1 user (show)

See Also:
Fixed By Commit(s):

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Lattner 2007-02-03 17:33:38 PST 
Consider:

@G = global i32 123
@ltmp_0_1 = global i32 123

define i32 @test(i32 *%G) {
        %A = load i32* %G
        %B = load i32* @ltmp_0_1
        %C = add i32 %A, %B
        ret i32 %C
}

The CBE generated code is:

unsigned int test(unsigned int *ltmp_0_1) {
...
  ltmp_1_2 = *ltmp_0_1;     // local
  ltmp_2_2 = *(&ltmp_0_1); // global
  return (ltmp_1_2 + ltmp_2_2);
...

This is a serious bug.

A not-as-serious bug: the CBE ignores the local names of symbols, turning %A into ltmp_1_2?

-Chris
Comment 1 Reid Spencer 2007-02-03 18:02:21 PST 
IIRC, the global/local patch was yours even though I committed it after getting
llvm-upgrade working. I would appreciate it if you could look into this as I am
behind on other tasks (the Shift thing set me back several days). If you don't
have time, this might have to wait a bit until I'm back on schedule. I have a
very important deadline happening in a few weeks .. can't miss it.
Comment 2 Chris Lattner 2007-02-05 12:46:33 PST 
btw, yes, I will do this, when I get time.

Thanks,

-Chris
Comment 3 Bill Wendling 2007-02-20 18:59:55 PST 
What would be a good strategy for naming variables? I could do something along these lines:

@var = global i32 927

define i32 @test(i32 *%var) {
  %A = load i32* %var
  %B = load i32* @var;
  %C = add i32 %A, %B
  ret i32 %C
}

into:

unsigned int test(unsigned int *param_var) {
...
  loc_A = *param_var;
  loc_B = *(&gbl_var);
  return (loc_A + loc_B);
}

This way, there is no ambiguity about what is local, global, or a parameter, there would be no name 
conflicts, and it uses the original names.

-bw
Comment 4 Chris Lattner 2007-02-20 19:53:16 PST 
I think using a prefix for local names makes sense.  How about "llvm_cbe_"?  It could be used for any local 
name (anything that is !isa<GlobalValue>) including params.

-Chris
Comment 5 Bill Wendling 2007-02-20 23:06:27 PST 
So then in the example, it would be something like:

  llvm_cbe_A = ...;
  llvm_cbe_B = ...;

?

I'm wondering if there's still a problem with name conflicts between globals and locals. What if a global's 
named "llvm_cbe_Foo" and the local's Foo also?

-bw
Comment 6 Chris Lattner 2007-02-20 23:32:06 PST 
> So then in the example, it would be something like:
>  llvm_cbe_A = ...;

yep

> I'm wondering if there's still a problem with name conflicts between globals and locals. What if a 
global's 
> named "llvm_cbe_Foo" and the local's Foo also?

Just adding a unique prefix like this makes it less likely.  To be extremely paranoid, the local symbols 
should be picked and checked to see if they conflict with any global ones.  If so, pick a different name.

-Chris
Comment 7 Bill Wendling 2007-02-21 15:38:23 PST 
How about this?

unsigned int G = ((unsigned int )123);
unsigned int ltmp_0_1 = ((unsigned int )123);

unsigned int test(unsigned int *llvm_cbe_G) {
  unsigned int llvm_cbe_A;
  unsigned int llvm_cbe_B;

  llvm_cbe_A = *llvm_cbe_G;
  llvm_cbe_B = *(&ltmp_0_1);
  return (llvm_cbe_A + llvm_cbe_B);
}
Comment 8 Chris Lattner 2007-02-21 17:13:25 PST 
looks great to me!